fix(create): remove nested monorepo lint config - #2667
Conversation
|
Can you add a new snapshot test case to cover this bug fix? |
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
dd80e94 to
e988fd6
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd80e94f48
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| continue; | ||
| } | ||
| let Some(parent_object) = node.parent() else { continue }; | ||
| if parent_object.kind() != "object" || !is_recognized_config_object(&parent_object) { |
There was a problem hiding this comment.
Restrict removal to direct config objects
When a defineConfig callback contains a nested function that returns an object with a lint property—for example, an inline plugin hook—is_recognized_config_object considers that returned object part of the Vite config because it accepts any return beneath the callback. Consequently, this destructive transform removes both the top-level lint configuration and the unrelated nested property. Use the existing is_direct_recognized_config_object predicate, which explicitly excludes nested-function returns, and cover this case with a regression test.
Useful? React with 👍 / 👎.
| : 'utils'; | ||
| const libraryProjectPath = path.join(fullPath, libraryDir); | ||
| setPackageName(libraryProjectPath, libraryPackageName); | ||
| removeNestedLibraryLintConfig(libraryProjectPath); |
There was a problem hiding this comment.
Apply lint removal to libraries added to existing monorepos
When vp create vite:library is run inside an existing monorepo, execution downloads the same LibraryTemplateRepo through executeBuiltinTemplate (packages/cli/src/create/templates/builtin.ts:33-52) and then goes directly through rewriteMonorepoProject (packages/cli/src/create/bin.ts:1229-1244); neither path calls this helper. That generated workspace member therefore retains the same unsupported nested lint block, so the regression remains for the normal add-a-library flow even though the initial vite:monorepo library is fixed. Invoke this cleanup from the shared library scaffold path when the destination is a monorepo member.
Useful? React with 👍 / 👎.
The standalone library template enables lint type checking, but nested lint configuration is not supported in monorepos. Remove the generated library's lint block during monorepo scaffolding while preserving it for standalone libraries. Apply the cleanup when adding libraries to existing monorepos as well. Use an AST-based config-key transform restricted to direct config objects, and add CLI snapshot coverage for both creation paths.
e988fd6 to
9932a48
Compare
|
@fengmk2 I added snapshot test coverage for this bug fix. Thanks for the suggestion! |
Summary
vite:libraryprojectsFixes #2640
Test plan
cargo test -p vp_migration remove_config_keycargo fmt --check -- crates/vp_migration/src/vite_config.rs crates/vp_migration/src/lib.rs packages/cli/binding/src/migration.rsgit diff --check