Conversation
|
|
WalkthroughThe pull request refactors various modules within the Nuxt package. Changes include adjusting type import statements by consolidating imports from the same module, and altering functions from asynchronous to synchronous implementations. Custom parsing logic relying on Acorn has been removed in favour of direct calls to streamlined parsing utilities. Functions related to metadata extraction, route processing, and tree-shaking are updated to eliminate the intermediate transformation steps. Test cases have been modified accordingly, where asynchronous handling is replaced with synchronous execution and dependency on Acorn is removed. These revisions simplify the control flow across components, core plugins, utilities, and tests without altering the overall functionality. 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
|
|
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
|
This is cool, love the utils. I was wondering if we could export these utils within @nuxt/kit as many modules have the same requirements and the API seems fairly stable. Can tackle as a separate PR if preferred |
|
Great idea! I've created (We could also create a workaround for the incorrect start/end positions with unicode text within that library.) |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
@nuxt/kit
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
CodSpeed Performance ReportMerging #30066 will not alter performanceComparing Summary
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/nuxt/test/treeshake-client.test.ts (1)
58-58: Simplified transform method callThe transform function call has been streamlined, removing unnecessary parsing context.
Consider replacing the generic
Functiontype with a more specific function signature type to improve type safety:-const result = await (treeshakeTemplatePlugin.transform! as Function)(source, 'test.ts') +const result = await (treeshakeTemplatePlugin.transform! as (code: string, id: string) => Promise<string | { code: string } | null>)(source, 'test.ts')🧰 Tools
🪛 Biome (1.9.4)
[error] 58-58: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
📜 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 (12)
packages/nuxt/src/components/plugins/tree-shake.ts(1 hunks)packages/nuxt/src/core/plugins/plugin-metadata.ts(2 hunks)packages/nuxt/src/core/utils/parse.ts(3 hunks)packages/nuxt/src/pages/plugins/page-meta.ts(1 hunks)packages/nuxt/src/pages/route-rules.ts(2 hunks)packages/nuxt/src/pages/utils.ts(4 hunks)packages/nuxt/test/component-names.test.ts(1 hunks)packages/nuxt/test/composable-keys.test.ts(1 hunks)packages/nuxt/test/page-metadata.test.ts(20 hunks)packages/nuxt/test/plugin-metadata.test.ts(3 hunks)packages/nuxt/test/route-rules.test.ts(2 hunks)packages/nuxt/test/treeshake-client.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/nuxt/src/components/plugins/tree-shake.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/nuxt/test/treeshake-client.test.ts
[error] 58-58: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-on, json, 18)
🔇 Additional comments (39)
packages/nuxt/src/pages/plugins/page-meta.ts (1)
215-215: Enhanced language detection for parsingThe code now appends a file extension based on
query.langor defaults to.tswhen parsing withparseAndWalk, which improves language-specific parsing accuracy.packages/nuxt/test/component-names.test.ts (1)
45-45: Simplified transform method callThe transform call has been streamlined by directly passing the content and filepath parameters, removing unnecessary complexity. This aligns with the broader changes to parsing logic throughout the codebase.
packages/nuxt/test/composable-keys.test.ts (3)
42-42: Simplified transform method callThe transform call has been streamlined by directly passing the code and filepath parameters, removing unnecessary complexity around parsing setup.
50-50: Simplified transform method callTransform method call now uses a direct approach without custom parsing logic.
58-58: Simplified transform method callTransform method call now uses a direct approach without custom parsing logic.
packages/nuxt/test/route-rules.test.ts (2)
6-8: Function updated to synchronous usage.This change aligns with the modification of
extractRouteRulesin the source file, which was converted from async to sync. The test now correctly calls the function withoutawait.
45-54: Test coverage improved with additional example.Good addition of a test case for
defineRouteRuleswithin a setup function, which increases test coverage for an important use case.packages/nuxt/test/page-metadata.test.ts (16)
14-16: Function updated to synchronous usage.The test case has been properly updated to call
getRouteMetasynchronously, aligning with the changes in the implementation.
19-26: Removed await from the function call.Test correctly updated to use the synchronous version of
getRouteMeta, eliminating theasync/awaitpattern.
29-42: Simplified function call.Test updated to directly use the synchronous
getRouteMetafunction, improving code clarity.
44-55: Removed asynchronous handling.Test properly updated to use the synchronous implementation of
getRouteMeta, streamlining the test execution.
57-76: Decorator support test updated.Test now correctly calls
getRouteMetasynchronously, and it's good to see a test case specifically for experimental decorators which oxc-parser natively supports.
78-86: Cache testing updated to synchronous implementation.Cache verification logic updated correctly to use the synchronous version of
getRouteMeta.
89-93: State isolation test updated.Test for verifying state isolation between calls to
getRouteMetaupdated to use the synchronous function.
95-135: Serialisable metadata extraction test updated.Test correctly updated to use the synchronous version of
getRouteMeta, maintaining the same validation logic.
137-170: Multiple blocks test updated.Test for extracting metadata from files with multiple script blocks updated to use the synchronous implementation.
172-198: Options API test updated.Test for extracting metadata in Options API context converted to synchronous implementation.
200-220: Quoted properties test updated.Test for handling quoted properties in metadata updated to use synchronous implementation.
222-238: Extra meta extraction test updated.Test for extracting configured extra metadata updated to use synchronous implementation.
242-286: Route normalization test updated.Test for normalizing routes with extracted metadata updated to use the synchronous implementation of
getRouteMeta.
552-587: Test for name preservation updated.Test updated to use synchronous transformation, and the snapshot correctly reflects the expected output for parsing with oxc-parser.
590-614: Await expression error test updated.Test for throwing errors on await expressions updated to call the transform function synchronously.
672-711: Reference identifiers test updated.Snapshot test for reference identifiers updated to match the expected output from oxc-parser processing.
packages/nuxt/src/core/plugins/plugin-metadata.ts (2)
10-10: Updated import statement.Explicitly importing
parseAndWalkandwithLocationsfrom the parse utility for direct usage, improving code clarity by showing direct dependencies.
41-49: Function converted from async to sync.The
extractMetadatafunction has been simplified by removing asynchronous handling. The function now directly processes the code usingparseAndWalkinstead of first transforming it, which streamlines the implementation and potentially improves performance.packages/nuxt/src/pages/route-rules.ts (3)
7-7: Simplified import statement.Updated import to only include
parseAndWalk, removing the unusedtransformfunction, which aligns with the simplified implementation.
13-13: Function converted from async to sync.The
extractRouteRulesfunction has been simplified by removing the asynchronous handling. This change improves code clarity and potentially performance by removing unnecessary promise handling.
29-33: Direct parsing of code.The function now directly parses the original code using
parseAndWalkinstead of first transforming it. This simplifies the implementation and improves the extraction of therulesStringby slicing from the original source.packages/nuxt/test/plugin-metadata.test.ts (3)
7-13: Well-organised test data structureExtracting the test properties into a structured object makes the test more maintainable and allows for better parameterisation.
14-26: Good use of parameterised testing with it.each()The refactoring from a traditional loop to a parameterised test with
it.each()improves test readability and maintainability. This approach:
- Makes each property test a separate test case
- Provides better failure reporting (shows exactly which property failed)
- Simplifies the test structure
Also, note that the
extractMetadatacall is now synchronous, matching the implementation changes in the core function.
39-39: Synchronous transform function usage aligns with implementation changesThe transform function is now being called synchronously, which is consistent with the changes to make parsing synchronous with the new
oxc-parser.packages/nuxt/src/core/utils/parse.ts (4)
3-4: Improved imports with oxc-parser adoptionThe switch from Acorn to
oxc-parseris a key change aligned with the PR objectives. Using named imports directly fromestreeand importingparseSyncfromoxc-parsermakes the dependencies more explicit.
15-15: Type improvement using ThisParameterTypeNice update to use the built-in
ThisParameterTypeutility type rather than a customInferThistype.
24-35: Simplified walk implementationThe implementation is now cleaner with unnecessary type casting removed. This makes the code more readable while maintaining type safety.
41-44: Improved parseAndWalk implementation with oxc-parserThe refactored synchronous implementation:
- Properly extracts the language type from filename
- Uses
parseSyncfromoxc-parserwith appropriate parameters- Accesses and returns the AST through
ast.programThis change aligns with the PR objective to transition from using Acorn to
oxc-parserdirectly, which should improve performance and add native support for decorators.packages/nuxt/src/pages/utils.ts (4)
14-14: Streamlined imports with removal of transform dependencyThe removal of the
transformimport reflects the transition to direct parsing without transformation steps.
210-210: Function signature changed from async to syncThe
getRouteMetafunction has been updated to be synchronous, removing the Promise return type. This matches the changes in the underlying parsing utilities and should improve performance.
241-241: Direct usage of parseAndWalk without awaitThe synchronous call to
parseAndWalkreflects the implementation change in the parsing utilities.
257-257: Using script.code directly without transformationThe code now uses
script.codedirectly instead of transformed code, which aligns with the more direct parsing approach usingoxc-parser.
🔗 Linked issue
resolves #29790
closes #30025
📚 Description
this moves from
acorn+esbuildto usingoxc-parserdirectly to parse code in our plugins. The main benefits are:⚡️ speed - this is faster and has the potential to be faster still - need to create some benchmarks to measure🎨 decorator support - this is natively supported byoxc-parser- we'll need to update feat(kit,nuxt,schema): support experimental decorators syntax #27672 to apply only to nitroThis PR also includes additional tests for the transformation plugins and fixes a few bugs I found along the way, including removing type assertions
and relying on the auto-generated AST types from.oxc🚧 TODO
oxc-parserwith unicode: unicode characters result in incorrect start/end values oxc-project/oxc#7508