Conversation
* shrink bundle size: - use `oxc-parser` for AST parsing - remove previous AST related packages - remove unused packages - update some packages * add tests for `compound-components` transformer * use oxc-parser types * infer types * move `removeReactImport` to it's own file, add tests * dedupe tests * enhance `addToConfig` to inline/multiline detection and indentation * infer types
🦋 Changeset detectedLatest commit: b6d3839 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR migrates the CLI tooling from recast/acorn-based AST parsing to oxc-parser, replaces tsup with tsdown as the bundler for the CLI package, updates multiple dependencies including Storybook (10.0.x → 10.2.4) and Bun (1.3.1 → 1.3.8), and refactors utilities like Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui/src/helpers/tailwind-merge.ts (1)
18-18:⚠️ Potential issue | 🔴 CriticalBug: version-to-import mapping is inverted.
When
version === 3, this selectsextendTailwindMerge_v2(fromtailwind-merge@2), and vice versa. This means Tailwind CSS v3 users get the v2 merge logic, and v2 users get the v3 merge logic — producing incorrect class merging behavior for all users.🐛 Proposed fix
- const twMergeFn = (version === 3 ? extendTailwindMerge_v2 : extendTailwindMerge_v3)({ + const twMergeFn = (version === 3 ? extendTailwindMerge_v3 : extendTailwindMerge_v2)({
🤖 Fix all issues with AI agents
In `@packages/ui/src/cli/transformers/compound-components.ts`:
- Around line 209-221: The rewrite of import specifiers currently discards
aliases by rebuilding the specifier list from names; update the logic around
newImportsToAdd/finalImportInfo so existing specifiers use their original source
text (use content.slice(specifier.start, specifier.end) from the stored
finalImportInfo.specifiers) while only appending newly added identifiers by
name; then build allSpecifiers from the preserved original specifier texts plus
the new import names before joining and calling
s.overwrite(finalImportInfo.specifiersStart, finalImportInfo.specifiersEnd,
newSpecifiersText), preserving multiline formatting logic as-is.
- Around line 136-166: The code overwrites flowbiteImportInfo for each matching
ImportDeclaration so only the last import receives new specifiers; update the
logic in importVisitor.ImportDeclaration to collect multiple import metadata
(e.g., change flowbiteImportInfo to an array like flowbiteImportInfos) or pick a
deterministic target import (prefer the base "flowbite-react" import over
"flowbite-react/components/...") when multiple matches exist, push each
specifier into flowbiteImportSpecifiers and also record/specify which
flowbiteImportInfo should receive new specifiers (or merge specifiers into the
preferred import entry); adjust later code that writes the rewritten import to
use the chosen flowbiteImportInfo entry (or consolidated entry) so all added
specifiers are inserted into the correct ImportDeclaration.
🧹 Nitpick comments (7)
packages/ui/src/cli/utils/remove-react-import.ts (1)
6-25:specifiers[0]check only matches default-import-only statements.The filter checks only
specifiers[0]?.local?.name === "React", so a combined import likeimport React, { useState } from "react"would have the entire declaration removed (losing named imports). This is fine for the current usage insetup-init.tswhere the generated content is controlled, but worth noting if this utility is intended to be general-purpose.packages/ui/src/cli/utils/add-plugin.test.ts (1)
197-203: Consider verifying plugin insertion order in the multi-plugin test.Other tests (lines 92, 111, 130, 152) use regex to verify that
flowbiteReact()is appended after existing plugins. This test only checks presence via[\s\S]*flowbiteReact\(\)[\s\S]*, not ordering relative totsConfigPaths,vitePluginRequire, orvuePlugin.packages/ui/scripts/generate-metadata.ts (1)
90-92: Consider logging parse errors instead of silently returning empty.Both
extractClassListandextractDependencyListreturn[]on parse errors with no diagnostic output. If a theme or component file has a syntax error, this silently produces incomplete metadata. Aconsole.warnwith the file context would help debugging build issues.💡 Proposed diagnostic logging
if (result.errors.length > 0) { + console.warn(`Failed to parse content, skipping. Errors: ${result.errors.map(e => e.message).join(", ")}`); return []; }packages/ui/src/cli/transformers/compound-components.test.ts (1)
904-1076: Consider adding a test for multipleflowbite-reactimport declarations.The edge cases section is solid, but there's no test for a file that has both
import { Accordion } from "flowbite-react"andimport { Modal } from "flowbite-react/components/Modal"simultaneously. This would exercise the "last import wins" behavior flagged in the transformer review.packages/ui/src/cli/utils/update-build-config.ts (2)
162-167: Hardcoded 8-space indentation when insertingpluginsproperty.Line 164 uses
",\n plugins: [${pluginName}]"(8 spaces), which won't match files using 2-space, 4-space, or tab indentation. Compare withadd-to-config.tswhich hasdetectIndentUnitfor this purpose.💡 Suggestion: reuse indent detection
+ const indent = detectIndentUnit(content); if (finalBuildOptions.hasProperties && finalBuildOptions.lastPropertyEnd) { - s.appendLeft(finalBuildOptions.lastPropertyEnd, `,\n plugins: [${pluginName}]`); + // Detect indentation from existing properties + const beforeObj = content.slice(Math.max(0, finalBuildOptions.objectStart), finalBuildOptions.lastPropertyEnd); + const indentMatch = beforeObj.match(/\n([ \t]+)[^\n]*$/); + const propIndent = indentMatch ? indentMatch[1] : ' '; + s.appendLeft(finalBuildOptions.lastPropertyEnd, `,\n${propIndent}plugins: [${pluginName}]`); } else { s.appendLeft(finalBuildOptions.objectStart + 1, ` plugins: [${pluginName}] `); }
57-61: Silent error handling differs fromcompound-components.ts.
compound-components.tslogsconsole.warn(...)on parse errors, but this function silently returns the original content. Minor inconsistency — consider adding aconsole.warnfor debuggability.packages/ui/src/cli/utils/add-to-config.ts (1)
449-508: Indent detection heuristic filters out indentation > 4 characters.Line 464 discards indentation strings longer than 4 characters. In files that are heavily nested (e.g., most lines at 6+ spaces), the 2-space base indent may appear infrequently and the heuristic could fall back to a wrong unit. This is unlikely for typical config files, but worth keeping in mind.
| const importVisitor = new Visitor({ | ||
| ImportDeclaration(node) { | ||
| if ( | ||
| node.type === "ImportDeclaration" && | ||
| (node.source.value === "flowbite-react" || node.source.value.startsWith("flowbite-react/components/")) && | ||
| Array.isArray(node.specifiers) && | ||
| node.specifiers.every((s) => s.type === "ImportSpecifier") | ||
| node.source?.value === "flowbite-react" || | ||
| (typeof node.source?.value === "string" && node.source.value.startsWith("flowbite-react/components/")) | ||
| ) { | ||
| flowbiteImportPath = node; | ||
| node.specifiers.forEach((specifier) => { | ||
| if (specifier.imported.type === "Identifier") { | ||
| flowbiteImportSpecifiers.push(specifier.imported.name); | ||
| if (Array.isArray(node.specifiers) && node.specifiers.every((s) => s.type === "ImportSpecifier")) { | ||
| const importText = content.slice(node.start, node.end); | ||
| const braceStart = importText.indexOf("{"); | ||
| const braceEnd = importText.lastIndexOf("}"); | ||
|
|
||
| if (braceStart !== -1 && braceEnd !== -1) { | ||
| flowbiteImportInfo = { | ||
| start: node.start, | ||
| end: node.end, | ||
| specifiersStart: node.start + braceStart + 1, | ||
| specifiersEnd: node.start + braceEnd, | ||
| specifiers: [], | ||
| }; | ||
|
|
||
| node.specifiers.forEach((specifier) => { | ||
| if (specifier.imported?.type === "Identifier") { | ||
| flowbiteImportSpecifiers.push(specifier.imported.name); | ||
| flowbiteImportInfo!.specifiers.push(specifier.imported.name); | ||
| } | ||
| }); | ||
| } | ||
| }); | ||
| } | ||
| } | ||
| return false; | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Multiple flowbite-react imports: only the last import declaration receives new specifiers.
flowbiteImportInfo is overwritten on each matching ImportDeclaration, so if a file has both import { Accordion } from "flowbite-react" and import { Modal } from "flowbite-react/components/Modal", new specifiers are appended only to whichever import was visited last. flowbiteImportSpecifiers is cumulative so JSX replacements still happen correctly, but the rewritten import block may not land in the expected declaration.
This is a narrow edge case for a migration tool, but worth noting.
🤖 Prompt for AI Agents
In `@packages/ui/src/cli/transformers/compound-components.ts` around lines 136 -
166, The code overwrites flowbiteImportInfo for each matching ImportDeclaration
so only the last import receives new specifiers; update the logic in
importVisitor.ImportDeclaration to collect multiple import metadata (e.g.,
change flowbiteImportInfo to an array like flowbiteImportInfos) or pick a
deterministic target import (prefer the base "flowbite-react" import over
"flowbite-react/components/...") when multiple matches exist, push each
specifier into flowbiteImportSpecifiers and also record/specify which
flowbiteImportInfo should receive new specifiers (or merge specifiers into the
preferred import entry); adjust later code that writes the rewritten import to
use the chosen flowbiteImportInfo entry (or consolidated entry) so all added
specifiers are inserted into the correct ImportDeclaration.
| if (newImportsToAdd.size > 0 && finalImportInfo) { | ||
| const allSpecifiers = [...finalImportInfo.specifiers, ...newImportsToAdd].sort((a, b) => a.localeCompare(b)); | ||
| const originalImportText = content.slice(finalImportInfo.specifiersStart, finalImportInfo.specifiersEnd); | ||
| const isMultiline = originalImportText.includes("\n"); | ||
|
|
||
| let newSpecifiersText: string; | ||
| if (isMultiline) { | ||
| newSpecifiersText = "\n " + allSpecifiers.join(",\n ") + ",\n"; | ||
| } else { | ||
| newSpecifiersText = " " + allSpecifiers.join(", ") + " "; | ||
| } | ||
|
|
||
| s.overwrite(finalImportInfo.specifiersStart, finalImportInfo.specifiersEnd, newSpecifiersText); |
There was a problem hiding this comment.
Import alias information is lost when rewriting specifiers.
If the original import used aliases (e.g., import { Accordion as Acc } from "flowbite-react"), the rewrite at Line 210 rebuilds the specifier list using only specifier.imported.name, discarding the alias. This would change the user's code semantics.
Again a narrow edge case for typical flowbite-react usage, but could silently break files with aliased imports.
💡 Sketch: preserve original specifier text instead of just the name
One approach is to store the original text of each specifier (via content.slice(specifier.start, specifier.end)) in flowbiteImportInfo.specifiers and only use identifier names for the newly added imports.
🤖 Prompt for AI Agents
In `@packages/ui/src/cli/transformers/compound-components.ts` around lines 209 -
221, The rewrite of import specifiers currently discards aliases by rebuilding
the specifier list from names; update the logic around
newImportsToAdd/finalImportInfo so existing specifiers use their original source
text (use content.slice(specifier.start, specifier.end) from the stored
finalImportInfo.specifiers) while only appending newly added identifiers by
name; then build allSpecifiers from the preserved original specifier texts plus
the new import names before joining and calling
s.overwrite(finalImportInfo.specifiersStart, finalImportInfo.specifiersEnd,
newSpecifiersText), preserving multiline formatting logic as-is.
1 participant
General
Bunversion to1.3.8apps/storybookpackagescreate-flowbite-reactDescription
Change bundler
Changes
tsuptotsdownflowbite-reactDescription
Shrink final bundle size from
42 MBto8.38 MBChanges
oxc-parserplugin/modernjsto the new APIbefore
after
Summary by CodeRabbit