diff --git a/.changeset/tiny-moons-hang.md b/.changeset/tiny-moons-hang.md new file mode 100644 index 0000000000..1ebdc1f862 --- /dev/null +++ b/.changeset/tiny-moons-hang.md @@ -0,0 +1,5 @@ +--- +"shadcn": patch +--- + +fix handling of nested aschild transforms diff --git a/packages/shadcn/src/utils/transformers/transform-aschild.test.ts b/packages/shadcn/src/utils/transformers/transform-aschild.test.ts index 5005f6e421..61fcb25e06 100644 --- a/packages/shadcn/src/utils/transformers/transform-aschild.test.ts +++ b/packages/shadcn/src/utils/transformers/transform-aschild.test.ts @@ -63,7 +63,7 @@ export function Component() { }) describe("DialogTrigger with non-Button child", () => { - test("transforms asChild to render prop with nativeButton={false}", async () => { + test("transforms asChild to render prop without nativeButton", async () => { expect( await transform( { @@ -86,7 +86,7 @@ export function Component() { export function Component() { return ( - } nativeButton={false}>Open Dialog + }>Open Dialog ) }" `) @@ -186,7 +186,7 @@ export function Component() { }) }) - describe("component with Link child", () => { + describe("Button with Link child", () => { test("transforms asChild to render prop with nativeButton={false}", async () => { expect( await transform( @@ -390,6 +390,173 @@ export function Component() { }) }) + describe("nested asChild", () => { + test("transforms inner asChild first, then outer", async () => { + expect( + await transform( + { + filename: "test.tsx", + raw: `import * as React from "react" + +export function Component() { + return ( + + + Home + + + ) +}`, + config: testConfig, + }, + [transformAsChild] + ) + ).toMatchInlineSnapshot(` + "import * as React from "react" + + export function Component() { + return ( + } />}>Home + ) + }" + `) + }) + + test("adds nativeButton={false} only on nested Button", async () => { + expect( + await transform( + { + filename: "test.tsx", + raw: `import * as React from "react" + +export function Component() { + return ( + + + + ) +}`, + config: testConfig, + }, + [transformAsChild] + ) + ).toMatchInlineSnapshot(` + "import * as React from "react" + + export function Component() { + return ( + } nativeButton={false} />}>Open + ) + }" + `) + }) + + test("transforms nested with sibling asChild elements", async () => { + expect( + await transform( + { + filename: "test.tsx", + raw: `import * as React from "react" + +export function Component() { + return ( +
+ + + Home + + + + + +
+ ) +}`, + config: testConfig, + }, + [transformAsChild] + ) + ).toMatchInlineSnapshot(` + "import * as React from "react" + + export function Component() { + return ( +
+ } />}>Home + }>Edit +
+ ) + }" + `) + }) + + test("transforms nested with self-closing inner child", async () => { + expect( + await transform( + { + filename: "test.tsx", + raw: `import * as React from "react" + +export function Component() { + return ( + + + + + + ) +}`, + config: testConfig, + }, + [transformAsChild] + ) + ).toMatchInlineSnapshot(` + "import * as React from "react" + + export function Component() { + return ( + } />}> + ) + }" + `) + }) + + test("transforms triple-nested asChild", async () => { + expect( + await transform( + { + filename: "test.tsx", + raw: `import * as React from "react" + +export function Component() { + return ( + + + + Home + + + + ) +}`, + config: testConfig, + }, + [transformAsChild] + ) + ).toMatchInlineSnapshot(` + "import * as React from "react" + + export function Component() { + return ( + } />} />}>Home + ) + }" + `) + }) + }) + describe("idempotency", () => { test("running twice produces same output", async () => { const input = `import * as React from "react" diff --git a/packages/shadcn/src/utils/transformers/transform-aschild.ts b/packages/shadcn/src/utils/transformers/transform-aschild.ts index 543deff3c3..6be86d2d2e 100644 --- a/packages/shadcn/src/utils/transformers/transform-aschild.ts +++ b/packages/shadcn/src/utils/transformers/transform-aschild.ts @@ -27,118 +27,142 @@ export const transformAsChild: Transformer = async ({ sourceFile, config }) => { return sourceFile } - // Collect all transformations first, then apply them in reverse order. - // This prevents issues with invalidated nodes when modifying the tree. - const transformations: TransformInfo[] = [] + // Process asChild elements iteratively, starting from leaf-level elements. + // Each iteration transforms only elements with no asChild descendants, + // ensuring inner transforms complete before outer ones read the tree. + const MAX_ITERATIONS = 10 + for (let i = 0; i < MAX_ITERATIONS; i++) { + const jsxElements = sourceFile.getDescendantsOfKind(SyntaxKind.JsxElement) - // Find all JSX elements with asChild attribute. - const jsxElements = sourceFile.getDescendantsOfKind(SyntaxKind.JsxElement) - - for (const jsxElement of jsxElements) { - const openingElement = jsxElement.getOpeningElement() - const asChildAttr = openingElement.getAttribute("asChild") - - if (!asChildAttr) { - continue - } - - const parentTagName = openingElement.getTagNameNode().getText() - const children = jsxElement.getJsxChildren() - - // Find the first JSX element child (skip whitespace/text). - const childElement = children.find( - (child) => - child.getKind() === SyntaxKind.JsxElement || - child.getKind() === SyntaxKind.JsxSelfClosingElement + // Find all JSX elements with asChild attribute. + const asChildElements = jsxElements.filter((el) => + el.getOpeningElement().getAttribute("asChild") ) - if (!childElement) { - // No child element found, just remove asChild. - asChildAttr.remove() - continue + if (asChildElements.length === 0) { + break } - // Get child element info. - let childTagName: string - let childProps: string - let childChildren: string - - if (childElement.getKind() === SyntaxKind.JsxSelfClosingElement) { - const selfClosing = childElement.asKindOrThrow( - SyntaxKind.JsxSelfClosingElement + // Filter to leaf-only: elements with no asChild descendants. + const leafElements = asChildElements.filter((el) => { + const descendants = el.getDescendantsOfKind(SyntaxKind.JsxElement) + return !descendants.some((d) => + d.getOpeningElement().getAttribute("asChild") ) - childTagName = selfClosing.getTagNameNode().getText() - childProps = selfClosing - .getAttributes() - .map((attr) => attr.getText()) - .join(" ") - childChildren = "" - } else { - const jsxChild = childElement.asKindOrThrow(SyntaxKind.JsxElement) - const openingEl = jsxChild.getOpeningElement() - childTagName = openingEl.getTagNameNode().getText() - childProps = openingEl - .getAttributes() - .map((attr) => attr.getText()) - .join(" ") - // Get the children's text content. - childChildren = jsxChild - .getJsxChildren() - .map((c) => c.getText()) - .join("") - } - - // Determine if we need nativeButton={false}. - // Add it when the child element is a non-button element. - const needsNativeButton = - ELEMENTS_REQUIRING_NATIVE_BUTTON_FALSE.includes(childTagName) - - transformations.push({ - parentElement: jsxElement, - parentTagName, - childTagName, - childProps, - childChildren, - needsNativeButton, }) - } - // Apply transformations in reverse order to preserve node validity. - for (const info of transformations.reverse()) { - const openingElement = info.parentElement.getOpeningElement() - const closingElement = info.parentElement.getClosingElement() + // Collect all transformations first, then apply them in reverse order. + // This prevents issues with invalidated nodes when modifying the tree. + const transformations: TransformInfo[] = [] - // Get existing attributes (excluding asChild). - const existingAttrs = openingElement - .getAttributes() - .filter((attr) => { - if (attr.getKind() === SyntaxKind.JsxAttribute) { - const jsxAttr = attr.asKindOrThrow(SyntaxKind.JsxAttribute) - return jsxAttr.getNameNode().getText() !== "asChild" - } - return true + for (const jsxElement of leafElements) { + const openingElement = jsxElement.getOpeningElement() + const asChildAttr = openingElement.getAttribute("asChild") + + if (!asChildAttr) { + continue + } + + const parentTagName = openingElement.getTagNameNode().getText() + const children = jsxElement.getJsxChildren() + + // Find the first JSX element child (skip whitespace/text). + const childElement = children.find( + (child) => + child.getKind() === SyntaxKind.JsxElement || + child.getKind() === SyntaxKind.JsxSelfClosingElement + ) + + if (!childElement) { + // No child element found, just remove asChild. + asChildAttr.remove() + continue + } + + // Get child element info. + let childTagName: string + let childProps: string + let childChildren: string + + if (childElement.getKind() === SyntaxKind.JsxSelfClosingElement) { + const selfClosing = childElement.asKindOrThrow( + SyntaxKind.JsxSelfClosingElement + ) + childTagName = selfClosing.getTagNameNode().getText() + childProps = selfClosing + .getAttributes() + .map((attr) => attr.getText()) + .join(" ") + childChildren = "" + } else { + const jsxChild = childElement.asKindOrThrow(SyntaxKind.JsxElement) + const openingEl = jsxChild.getOpeningElement() + childTagName = openingEl.getTagNameNode().getText() + childProps = openingEl + .getAttributes() + .map((attr) => attr.getText()) + .join(" ") + // Get the children's text content. + childChildren = jsxChild + .getJsxChildren() + .map((c) => c.getText()) + .join("") + } + + // Determine if we need nativeButton={false}. + // Only add it on Button when the child is a non-button element. + const needsNativeButton = + parentTagName === "Button" && + ELEMENTS_REQUIRING_NATIVE_BUTTON_FALSE.includes(childTagName) + + transformations.push({ + parentElement: jsxElement, + parentTagName, + childTagName, + childProps, + childChildren, + needsNativeButton, }) - .map((attr) => attr.getText()) - .join(" ") - - // Build the render prop value. - const renderValue = info.childProps - ? `{<${info.childTagName} ${info.childProps} />}` - : `{<${info.childTagName} />}` - - // Build new attributes. - let newAttrs = existingAttrs ? `${existingAttrs} ` : "" - newAttrs += `render=${renderValue}` - if (info.needsNativeButton) { - newAttrs += ` nativeButton={false}` } - // Build the new element text. - const newChildren = info.childChildren.trim() ? `${info.childChildren}` : "" + // Apply transformations in reverse order to preserve node validity. + for (const info of transformations.reverse()) { + const openingElement = info.parentElement.getOpeningElement() - const newElementText = `<${info.parentTagName} ${newAttrs}>${newChildren}` + // Get existing attributes (excluding asChild). + const existingAttrs = openingElement + .getAttributes() + .filter((attr) => { + if (attr.getKind() === SyntaxKind.JsxAttribute) { + const jsxAttr = attr.asKindOrThrow(SyntaxKind.JsxAttribute) + return jsxAttr.getNameNode().getText() !== "asChild" + } + return true + }) + .map((attr) => attr.getText()) + .join(" ") - info.parentElement.replaceWithText(newElementText) + // Build the render prop value. + const renderValue = info.childProps + ? `{<${info.childTagName} ${info.childProps} />}` + : `{<${info.childTagName} />}` + + // Build new attributes. + let newAttrs = existingAttrs ? `${existingAttrs} ` : "" + newAttrs += `render=${renderValue}` + if (info.needsNativeButton) { + newAttrs += ` nativeButton={false}` + } + + // Build the new element text. + const newChildren = info.childChildren.trim() + ? `${info.childChildren}` + : "" + + const newElementText = `<${info.parentTagName} ${newAttrs}>${newChildren}` + + info.parentElement.replaceWithText(newElementText) + } } return sourceFile