feat(nuxt,schema): extract asyncData handlers to chunks #33131
Conversation
|
|
@nuxt/kit
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
CodSpeed Performance ReportMerging #33131 will not alter performanceComparing Summary
Footnotes |
|
I've tried the latest Does not workAxios for bundle size ofc. import axios from 'axios'
const { data } = await useAsyncData('index', fetcherFn)
async function fetcherFn() {
// Create a function that has a big bundle size
return axios.get('/api/hello').then(res => res.data)
}Does workimport axios from 'axios'
const { data } = await useAsyncData('index', () => axios.get('/api/hello').then(res => res.data)) |
Walkthrough
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
docs/2.guide/3.going-further/1.experimental-features.md(1 hunks)packages/nuxt/src/core/nuxt.ts(2 hunks)packages/nuxt/src/core/plugins/extract-async-data-handlers.ts(1 hunks)packages/nuxt/test/extract-async-data-handlers.test.ts(1 hunks)packages/nuxt/test/tree-shake.test.ts(1 hunks)packages/nuxt/test/utils.ts(1 hunks)packages/schema/src/config/experimental.ts(1 hunks)packages/schema/src/types/schema.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/nuxt/test/tree-shake.test.tspackages/nuxt/test/utils.tspackages/schema/src/config/experimental.tspackages/schema/src/types/schema.tspackages/nuxt/src/core/nuxt.tspackages/nuxt/test/extract-async-data-handlers.test.tspackages/nuxt/src/core/plugins/extract-async-data-handlers.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Write unit tests for core functionality using
vitest
Files:
packages/nuxt/test/tree-shake.test.tspackages/nuxt/test/extract-async-data-handlers.test.ts
🧬 Code graph analysis (2)
packages/nuxt/src/core/nuxt.ts (2)
packages/kit/src/build.ts (1)
addBuildPlugin(153-165)packages/nuxt/src/core/plugins/extract-async-data-handlers.ts (1)
ExtractAsyncDataHandlersPlugin(20-171)
packages/nuxt/test/extract-async-data-handlers.test.ts (2)
packages/nuxt/src/core/plugins/extract-async-data-handlers.ts (1)
ExtractAsyncDataHandlersPlugin(20-171)packages/nuxt/test/utils.ts (1)
clean(9-18)
🪛 ast-grep (0.39.6)
packages/nuxt/test/utils.ts
[warning] 15-15: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^\\s{${indent}})
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🪛 Biome (2.1.2)
packages/nuxt/src/core/plugins/extract-async-data-handlers.ts
[error] 8-8: Do not shadow the global "Function" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🪛 GitHub Actions: docs
packages/schema/src/types/schema.ts
[error] 1-1: Network error: Vue API fetch failed. https://vuejs.org/api/application.html#app-config
| const referencedVariables = new Set<string>() | ||
| const imports = new Set<string>() | ||
|
|
||
| // Walk the function body to find all identifiers | ||
| walk(fetcherFunction.body, { | ||
| scopeTracker, | ||
| enter (innerNode) { | ||
| if (innerNode.type !== 'Identifier') { | ||
| return | ||
| } | ||
|
|
||
| const declaration = scopeTracker.getDeclaration(innerNode.name) | ||
| if (!declaration) { | ||
| return | ||
| } | ||
|
|
||
| if (declaration.type === 'Import') { | ||
| // This is an imported variable, we need to include the import | ||
| imports.add(innerNode.name) | ||
| } else if (declaration.type !== 'FunctionParam') { | ||
| const functionBodyStart = fetcherFunction.body!.start | ||
| const functionBodyEnd = fetcherFunction.body!.end | ||
|
|
||
| // If the declaration is not within the function body, it's external | ||
| if (declaration.start < functionBodyStart || declaration.end > functionBodyEnd) { | ||
| referencedVariables.add(innerNode.name) | ||
| } | ||
| } | ||
| }, | ||
| }) | ||
|
|
||
| // Collect import statements for the referenced imports | ||
| const importStatements = new Set<string>() | ||
| walk(parseResult.program, { | ||
| enter (importDecl) { | ||
| if (importDecl.type !== 'ImportDeclaration') { | ||
| return | ||
| } | ||
|
|
||
| // Check if this import declaration contains any of our referenced imports | ||
| if (importDecl.specifiers?.some(spec => spec.local && imports.has(spec.local.name))) { | ||
| importStatements.add(script.slice(importDecl.start, importDecl.end)) | ||
| } | ||
| }, | ||
| }) | ||
|
|
||
| const imps = Array.from(importStatements).join('\n') | ||
|
|
||
| // Generate a unique key for the extracted chunk | ||
| const key = `${dirname(id)}/async-data-chunk-${count++}.js` | ||
|
|
||
| // Get the function body content | ||
| const isBlockStatement = fetcherFunction.body.type === 'BlockStatement' | ||
|
|
||
| const startOffset = codeIndex + fetcherFunction.body.start | ||
| const endOffset = codeIndex + fetcherFunction.body.end | ||
|
|
||
| // Create the extracted chunk | ||
| const chunk = s.clone() | ||
| const parameters = [...referencedVariables].join(', ') | ||
| const returnPrefix = isBlockStatement ? '' : 'return ' | ||
| const preface = `${imps}\nexport default async function (${parameters}) { ${returnPrefix}` | ||
| const suffix = ` }` | ||
|
|
||
| if (isBlockStatement) { | ||
| // For block statements, we need to extract the content inside the braces | ||
| chunk.overwrite(0, startOffset + 1, preface) | ||
| chunk.overwrite(endOffset - 1, code.length, suffix) | ||
| } else { | ||
| // For expression bodies, wrap in return statement | ||
| chunk.overwrite(0, startOffset, preface) | ||
| chunk.overwrite(endOffset, code.length, suffix) | ||
| } | ||
|
|
||
| asyncDatas[key] = { | ||
| code: chunk.toString(), | ||
| map: options.sourcemap ? chunk.generateMap({ hires: true }) : undefined, | ||
| } | ||
|
|
||
| // Replace the original function with a dynamic import | ||
| const importCall = `() => import('${key}').then(r => (r.default || r)(${parameters}))` | ||
| s.overwrite(codeIndex + fetcherFunction.start, codeIndex + fetcherFunction.end, importCall) |
There was a problem hiding this comment.
Avoid capturing property identifiers (breaks destructuring destructure cases)
For code like:
const { data } = await useAsyncData('key', async () => {
const response = await $fetch('/api/data')
return response.data
})the transform currently becomes:
const { data } = await useAsyncData('key', () =>
import('…/async-data-chunk-0.js').then(r => (r.default || r)(data))
)Because the walker treats the property name data in response.data as a free identifier, we add it to referencedVariables and pass it into the chunk. Evaluating that argument reads the outer data binding while it is still inside the TDZ, so the call throws ReferenceError: Cannot access 'data' before initialization.
Please ignore identifiers that are acting as property keys (e.g. MemberExpression with computed === false, object literal keys, etc.) before calling scopeTracker.getDeclaration, so we only capture true free variables. Once updated, add a regression test ensuring the transform returns () => import(...).then(r => (r.default || r)()) for the snippet above.
🤖 Prompt for AI Agents
In packages/nuxt/src/core/plugins/extract-async-data-handlers.ts around lines 75
to 156, the identifier walker currently treats property names (e.g. the "data"
in response.data or object literal keys) as free identifiers and captures them,
causing TDZ ReferenceErrors; modify the enter handler to first inspect the
identifier's parent and skip calling scopeTracker.getDeclaration for identifiers
that are property keys (e.g. parent.type === 'MemberExpression' &&
parent.property === innerNode && parent.computed === false, parent.type ===
'Property' && parent.key === innerNode && parent.computed === false, class
MethodDefinition keys, and similar non-computed key positions), only then
proceed to call scopeTracker.getDeclaration for true variable references; after
the change add a regression test for the snippet in the comment asserting the
transform produces () => import(...).then(r => (r.default || r)()) (i.e., no
captured parameters).
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/nuxt/src/core/plugins/extract-async-data-handlers.ts (1)
8-8: Rename imported type to avoid shadowing global.The
Functiontype import shadows the built-in globalFunctionconstructor, which can cause confusion when reading the code. Consider using a type alias to avoid this.As per static analysis hints.
Apply this diff to resolve the shadowing:
-import type { ArrowFunctionExpression, Function } from 'oxc-parser' +import type { ArrowFunctionExpression, Function as ASTFunction } from 'oxc-parser'Then update the usage on line 67:
-const fetcherFunction = callExpression.arguments.find((fn): fn is Function | ArrowFunctionExpression => fn.type === 'ArrowFunctionExpression' || fn.type === 'FunctionExpression') +const fetcherFunction = callExpression.arguments.find((fn): fn is ASTFunction | ArrowFunctionExpression => fn.type === 'ArrowFunctionExpression' || fn.type === 'FunctionExpression')And on line 69:
-if (!fetcherFunction || (fetcherFunction.type !== 'ArrowFunctionExpression' && fetcherFunction.type !== 'FunctionExpression') || !fetcherFunction.body) { +if (!fetcherFunction || (fetcherFunction.type !== 'ArrowFunctionExpression' && fetcherFunction.type !== 'FunctionExpression') || !fetcherFunction.body) {Note: Line 69 check remains unchanged as it uses string literals, not the type.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/2.guide/3.going-further/1.experimental-features.md(1 hunks)packages/nuxt/src/core/nuxt.ts(2 hunks)packages/nuxt/src/core/plugins/extract-async-data-handlers.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/nuxt/src/core/nuxt.ts
- docs/2.guide/3.going-further/1.experimental-features.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/nuxt/src/core/plugins/extract-async-data-handlers.ts
🪛 Biome (2.1.2)
packages/nuxt/src/core/plugins/extract-async-data-handlers.ts
[error] 8-8: Do not shadow the global "Function" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: test-fixtures (windows-latest, built, rspack, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, dev, vite, default, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, built, vite, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, async, manifest-on, js, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-on, js, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, async, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, default, manifest-on, json, lts/-1)
- GitHub Check: typecheck (ubuntu-latest, bundler)
- GitHub Check: test-benchmark
- GitHub Check: test-size
- GitHub Check: typecheck (windows-latest, bundler)
- GitHub Check: code
🔇 Additional comments (4)
packages/nuxt/src/core/plugins/extract-async-data-handlers.ts (4)
86-104: Property identifier filtering correctly implemented.The fix for the TDZ issue raised in the previous review has been properly implemented. The code now correctly skips identifiers that act as property keys (member expressions, object literal keys, method definitions, and property definitions) before checking if they are free variables. This prevents incorrect capture of variables such as
datainresponse.datawhen the outer scope has a destructureddatabinding.
106-122: Dependency analysis logic is sound.The distinction between imports, function parameters, and external variables is handled correctly:
- Imported identifiers are collected for re-injection into the chunk
- Function parameters are excluded (as they're locally scoped)
- External variables (declared outside the function body) are identified by comparing declaration positions with function body boundaries
147-167: Chunk extraction logic handles both function body types correctly.The implementation properly distinguishes between block statements and expression bodies:
- Block statements: Extracts content inside braces
- Expression bodies: Wraps in return statement
The use of
MagicString.clone()correctly preserves the original transformation state whilst creating independent chunks.
174-176: Dynamic import replacement is correctly structured.The replacement correctly wraps the extracted chunk in a dynamic import:
- Returns an arrow function for lazy evaluation
- Passes referenced variables as arguments to match the extracted function's parameters
- Uses
r.default || ras a defensive fallback (though default should always exist given the export structure)
🔗 Linked issue
📚 Description
this ports https://github.com/danielroe/nuxt-rebundle to an experimental feature in Nuxt. (we successfully tested it on https://nuxt.com for a while before accidentally dropping it in nuxt/nuxt.com#1795.)
This extracts handler functions from
useAsyncDataanduseLazyAsyncDatacalls into separate chunks for improved code splitting and caching efficiency.This feature transforms inline handler functions into dynamically imported chunks:
The benefit of this transformation is that we can split out data fetching logic — while still allowing the code to be loaded if required.
Important
This feature is only recommended for static builds with payload extraction enabled, where data does not need to be re-fetched at runtime.