refactor(nuxt): migrate to oxc-walker
#32250
Conversation
|
|
|
@cernymatej looks like a few tests fail owing to differences in scope tracker - might warrant a look |
|
noticed that too… I'll investigate Locally, some tests related to logging failed too - specifically the ones that require the vite plugin defined in root-level nuxt.config.ts (intoduced in the async data layer rewrite). |
|
you should be able to run |
|
That's what I've been using, but for example, and if I change the const isDev = import.meta.dev /* and in test */ || true |
# Conflicts: # packages/nuxt/package.json # pnpm-lock.yaml
@nuxt/kit
nuxt
@nuxt/schema
@nuxt/rspack-builder
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
CodSpeed Performance ReportMerging #32250 will not alter performanceComparing Summary
|
|
The problem was that this PR didn't include the last-minute changes made in |
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the Nuxt parse utilities to use the new oxc-walker package and removes the old implementation. Key changes include updating imports and API calls across multiple files, replacing withLocations call usages with direct node references, and renaming options (e.g. keepExitedScopes to preserveExitedScopes).
Reviewed Changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/nuxt/src/pages/utils.ts | Migrated parse functions to use oxc-walker and updated types. |
| packages/nuxt/src/pages/route-rules.ts | Updated import and direct usage of parseAndWalk. |
| packages/nuxt/src/pages/plugins/page-meta.ts | Replaced withLocations & isNotReferencePosition with new methods. |
| packages/nuxt/src/head/plugins/unhead-imports.ts | Removed withLocations usage and switched to oxc-walker import. |
| packages/nuxt/src/core/plugins/prehydrate.ts | Modified parseAndWalk and removed withLocations from callback. |
| packages/nuxt/src/core/plugins/plugin-metadata.ts | Updated AST node handling and type adjustments. |
| packages/nuxt/src/core/plugins/composable-keys.ts | Replaced keepExitedScopes with preserveExitedScopes. |
| packages/nuxt/src/components/plugins/tree-shake.ts | Revised AST traversal and node removal logic using oxc-walker. |
| packages/nuxt/src/components/plugins/component-names.ts | Simplified import and node location retrieval. |
| packages/nuxt/package.json | Added dependency on oxc-walker. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (3)
packages/nuxt/src/pages/plugins/page-meta.ts:192
- Replacing 'isNotReferencePosition' with 'isBindingIdentifier' may change the filtering logic; please verify that this new condition preserves the intended behavior when handling identifier nodes.
if (isBindingIdentifier(node, parent) || node.type !== 'Identifier') { return }
packages/nuxt/src/core/plugins/composable-keys.ts:59
- [nitpick] Confirm that 'preserveExitedScopes' is the correct replacement for 'keepExitedScopes' in the current context, and update any related documentation if necessary.
preserveExitedScopes: true,
packages/nuxt/src/components/plugins/tree-shake.ts:51
- [nitpick] Verify that destructuring 'program' from the parseAndWalk result aligns with the new API's return structure and does not affect downstream AST processing.
const { program: ast } = parseAndWalk(code, id, (node) => {
WalkthroughThis set of changes migrates the codebase from using custom AST parsing, walking, and scope tracking utilities to relying on the external 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (1)
✨ 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/nuxt/package.jsonis excluded by!**/package.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (11)
packages/nuxt/src/components/plugins/component-names.ts(2 hunks)packages/nuxt/src/components/plugins/tree-shake.ts(7 hunks)packages/nuxt/src/core/plugins/composable-keys.ts(2 hunks)packages/nuxt/src/core/plugins/plugin-metadata.ts(4 hunks)packages/nuxt/src/core/plugins/prehydrate.ts(2 hunks)packages/nuxt/src/core/utils/parse.ts(1 hunks)packages/nuxt/src/head/plugins/unhead-imports.ts(2 hunks)packages/nuxt/src/pages/plugins/page-meta.ts(6 hunks)packages/nuxt/src/pages/route-rules.ts(1 hunks)packages/nuxt/src/pages/utils.ts(5 hunks)packages/nuxt/test/parse.test.ts(0 hunks)
💤 Files with no reviewable changes (1)
- packages/nuxt/test/parse.test.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/nuxt/src/pages/plugins/page-meta.ts (1)
packages/nuxt/src/pages/utils.ts (1)
isSerializable(630-676)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: codeql (javascript-typescript)
🔇 Additional comments (16)
packages/nuxt/src/core/utils/parse.ts (1)
7-9: Good implementation of the transform wrapper.The transform function correctly wraps esbuild's transform with Nuxt-specific options, maintaining backwards compatibility whilst simplifying the module's responsibilities.
packages/nuxt/src/pages/route-rules.ts (1)
6-6:❓ Verification inconclusive
Import change looks correct.
The migration from local
parseAndWalktooxc-walkerpackage is consistent with the overall migration strategy. The usage pattern in the file appears compatible with the new API.
🏁 Script executed:
#!/bin/bash # Description: Verify that parseAndWalk from oxc-walker has compatible API with the usage # Expected: Confirm that oxc-walker's parseAndWalk supports the same callback pattern # Check how parseAndWalk is used in this file rg -A 10 -B 2 "parseAndWalk" packages/nuxt/src/pages/route-rules.ts # Find other files using parseAndWalk to verify consistent migration rg -l "parseAndWalk.*oxc-walker" --type tsLength of output: 1517
Ensure oxc-walker parseAndWalk API compatibility
The import of
parseAndWalkfromoxc-walkeris correct and all usages in this package (includingpackages/nuxt/src/pages/route-rules.tsand six other files) follow the same signature:•
parseAndWalk(code, ext, callback)
• Inside the callback you rely onnode.startandnode.endPlease verify that the public API of
oxc-walker:
- Exports
parseAndWalk(code: string, ext: string, cb: (node: Node) => void)- Guarantees that
Nodeobjects includestartandendpropertiesReview the oxc-walker documentation or its TypeScript definitions to confirm before merging.
packages/nuxt/src/core/plugins/prehydrate.ts (2)
5-6: Clean migration to oxc-walker with improved simplicity.The import changes correctly migrate to
oxc-walkerwhilst maintaining thetransformimport from the local module (which is still available). The removal ofwithLocationswrapper simplifies the code by using direct node properties.
28-28: Simplified node property access.Good removal of the
withLocationswrapper. Direct access tonode.arguments[0]is cleaner and should work consistently withoxc-walker's AST nodes.packages/nuxt/src/components/plugins/component-names.ts (2)
4-4: Correct migration to oxc-walker.The import change from local parsing utilities to
oxc-walkeris consistent with the overall migration strategy.
50-50: Improved code clarity with direct property access.Excellent simplification! Direct destructuring of
startandendfromnode.declarationis much cleaner than using thewithLocationswrapper and improves code readability.packages/nuxt/src/head/plugins/unhead-imports.ts (1)
7-7: LGTM! Clean migration to oxc-walker.The removal of
withLocationswrapper and direct access to node properties is consistent with the new parser API.Also applies to: 58-59
packages/nuxt/src/core/plugins/composable-keys.ts (1)
9-9: LGTM! Correct API adjustments for oxc-walker.The option rename to
preserveExitedScopesand the updated return structure handling (parseResult.program) are consistent with the new parser API.Also applies to: 59-59, 61-62, 67-67
packages/nuxt/src/pages/plugins/page-meta.ts (1)
9-16:❓ Verification inconclusive
Verify the semantic equivalence of
isBindingIdentifier.The migration looks correct. However, please ensure that
isBindingIdentifierfrom oxc-walker provides the same semantic behaviour as the previousisNotReferencePositioncheck to avoid any unintended changes in identifier handling logic.Run the following script to verify that the identifier handling tests pass with the new implementation:
Also applies to: 178-178, 194-194, 216-216, 231-231, 239-239, 245-245, 263-263
🏁 Script executed:
#!/bin/bash # Description: Check if there are any tests specifically for identifier handling in page-meta that might catch semantic differences # Search for test files related to page-meta fd -e test.ts -e spec.ts | xargs rg -l "page-meta|definePageMeta" -A 5 # Search for any tests that might test identifier handling rg -A 10 "isNotReferencePosition|isBindingIdentifier|identifier.*handling" --type tsLength of output: 3207
Verify
isBindingIdentifiermatches previousisNotReferencePositionsemantics
No existing tests inpage-metareferenceisNotReferencePositionorisBindingIdentifier, so we can’t be sure the new check hasn’t altered identifier handling. Please manually confirm (or add tests to cover) that every case covered by the oldisNotReferencePosition—for example:
- variable declarations vs references
- function parameters and default values
- object destructuring and property accesses
- import specifiers
still behaves identically under
isBindingIdentifier.packages/nuxt/src/components/plugins/tree-shake.ts (1)
8-9: LGTM! Comprehensive type migration to oxc-parser.The migration from ESTree types to oxc-parser types is thorough and consistent. The updated filtering logic properly handles the new
BindingPatternandBindingPropertytypes.Also applies to: 51-51, 64-65, 67-67, 150-150, 173-173, 176-176, 240-240, 253-253, 259-259, 266-266
packages/nuxt/src/core/plugins/plugin-metadata.ts (3)
1-1: Import changes look good.The migration correctly adds the necessary imports from
oxc-walkerandoxc-parser. TheLiteraltype fromestreeis still needed for the type cast at line 109.Also applies to: 10-11
87-94: Function updates correctly handle new AST node types.The changes appropriately handle the migration from estree to oxc-parser AST nodes, with proper type checks for both string keys and IdentifierName objects.
168-170: Direct node property access is implemented correctly.The removal of the
withLocationswrapper and direct access tostartandendproperties aligns with the oxc-parser node structure.packages/nuxt/src/pages/utils.ts (3)
13-14: Import migration is correct.The replacement of estree types with oxc-parser types and the use of
parseAndWalkfrom oxc-walker aligns with the migration objectives.
260-263: Type updates in getRouteMeta are correct.The property finding logic correctly uses the new
ObjectPropertytype, and the direct access toproperty.valuealigns with the oxc-parser node structure.
631-676: isSerializable function correctly migrated to new AST types.All node type checks and property accesses have been properly updated to work with oxc-parser nodes. The direct access to
start,end, andvalueproperties is consistent with the removal of thewithLocationswrapper.
oxc-walker
Co-authored-by: Daniel Roe <daniel@roe.dev>
📚 Description
This PR removes the old implementation of the parse utils and migrates to the new ones from
oxc-walker.The dependency wasn't included yet, since this needs to be released first.