refactor(setup-init): relax checks on React imports #1596
Conversation
…ters removing it if not necessary - filter out `import React from "react"` from the AST when parsing current and new content
🦋 Changeset detectedLatest commit: 2033bb9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 Git ↗︎
|
WalkthroughThis update refactors the setup initialization process for the "flowbite-react" package by introducing logic to ignore the presence or absence of the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant ASTParser
User->>CLI: Run setup-init
CLI->>ASTParser: Parse current file to AST
CLI->>ASTParser: Parse new content to AST
CLI->>CLI: removeReactImport(currentAST)
CLI->>CLI: removeReactImport(newAST)
CLI->>CLI: Compare filtered ASTs
CLI-->>User: Proceed with update if needed
Estimated code review effort2 (~15 minutes) Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/ui/src/cli/commands/setup-init.tsOops! Something went wrong! :( ESLint: 8.57.0 ESLint couldn't find the plugin "eslint-plugin-react". (The package "eslint-plugin-react" was not found when loaded as a Node module from the directory "/packages/ui".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react" was referenced from the config file in "packages/ui/.eslintrc.cjs". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/ui/src/cli/commands/setup-init.ts (1)
77-91: Review the filtering logic for potential edge cases.The implementation correctly filters out
import React from "react"statements, but there are a few considerations:
- The logic assumes the first specifier is the default import for React - this is correct for the target case
- The function mutates the AST in place, which could have side effects if the AST is used elsewhere
- The filtering only handles default imports (
import React from "react"), not named imports or namespace importsConsider these improvements:
function removeReactImport(ast: namedTypes.File): namedTypes.File { + // Create a shallow copy to avoid mutating the original AST + const astCopy = { ...ast, program: { ...ast.program } }; + - if (ast?.program?.body) { - ast.program.body = ast.program.body.filter( + if (astCopy?.program?.body) { + astCopy.program.body = astCopy.program.body.filter( (node) => !( node.type === "ImportDeclaration" && "value" in node.source && typeof node.source.value === "string" && node.source.value === "react" && + // More robust check for React default import - node.specifiers?.[0]?.local?.name === "React" + node.specifiers?.some(spec => + spec.type === "ImportDefaultSpecifier" && + spec.local?.name === "React" + ) ), ); } - return ast; + return astCopy; }However, the current implementation is sufficient for the stated use case of handling IDE formatter behavior with React imports.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/cute-breads-attack.md(1 hunks)packages/ui/src/cli/commands/setup-init.ts(3 hunks)
🔇 Additional comments (3)
packages/ui/src/cli/commands/setup-init.ts (2)
2-2: LGTM! Appropriate import addition.The addition of
namedTypesimport from "ast-types" is necessary for the type annotation in the new helper function.
53-54: LGTM! Consistent application of React import filtering.The helper function is correctly applied to both the current and new content ASTs before comparison, ensuring consistent behavior regardless of IDE formatting preferences.
.changeset/cute-breads-attack.md (1)
1-8: LGTM! Well-documented changeset.The changeset correctly documents this as a patch-level change and accurately describes the refactoring to relax React import checks. The description clearly explains both the problem (IDE formatters removing unnecessary imports) and the solution (filtering out React imports from AST comparison).
Description
Relax checks on React imports due to IDE formatters removing it if not necessary which leads to formatting conflict.
Changes
import React from "react"from the AST when parsing current and new contentSummary by CodeRabbit