-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
feat(nuxt,kit,schema): add a factory function for useFetch and useAsyncData
#32300
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
|
@nuxt/kit
@nuxt/nitro-server
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
CodSpeed Performance ReportMerging #32300 will not alter performanceComparing Summary
|
|
@cernymatej It would also be great to support creating a custom |
|
@maximepvrt that is already possible in using https://nuxt.com/docs/guide/recipes/custom-usefetch#custom-fetch |
|
@cernymatej Yes, you're right. Tt's possible via It would be great if we could also easily create a custom export const useClientFetch = createUseFetch({ fetch: $api })or export const useClientFetch = createUseFetch($api) |
|
You can already register the custom // utils/myFetch.ts
export const $myFetch = $fetch.create() // will be auto-importedas for providing a custom // composables/myUseFetch.ts
import { $myFetch } from '../utils/myFetch'
export const myUseFetch = createUseFetch({ $fetch: $myFetch }) |
|
Perfect 🥰🥰 |
In numerous Nuxt projects, I always need to merge these hooks. For example, I log the error in This can be granular and on the hook level too!
Maybe this can be achieved by adding a |
…ng a compiler scan plugin
# Conflicts: # packages/kit/src/resolve.ts # packages/nuxt/src/utils.ts
# Conflicts: # packages/nuxt/package.json # packages/nuxt/src/app/composables/fetch.ts # packages/nuxt/src/components/module.ts # packages/nuxt/src/core/nuxt.ts # packages/schema/src/index.ts # pnpm-lock.yaml
…ed function source check
| })) | ||
| } | ||
|
|
||
| // TODO: add support for HMR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HMR support is also a little tricky... the scan plugins adjust nuxt.options.optimization.keyedComposables 🤔
afterScan: (nuxt) => {
nuxt.options.optimization.keyedComposables.push(...keyedFunctionsCreatedByFactories)
},Is there a clever way to handle this without having to restart Nuxt?
| await nuxt.callHook('modules:done') | ||
|
|
||
| await nuxt.callHook('compiler:ready') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we use a different hook? the scanning can't run in modules:done because that's the hook where imports:context is called, and the unimport context is needed for scanning
Does this mean we'll be able to pass/augment default types for body, error, and options ? |
WalkthroughThis pull request introduces a factory-based system for data-fetching composables and a compiler infrastructure to support them. It adds Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
♻️ Duplicate comments (1)
packages/nuxt/src/compiler/module.ts (1)
204-205: HMR: re-scan and refresh keyed composables on file changes.Wire a dev-only watcher over normalised scan dirs; on add/change/remove, re-run scan plugins, diff
nuxt.options.optimization.keyedComposables, and invalidate the keyed‑functions build plugin cache to avoid restart. Consider a throttle/debounce to batch updates.
🧹 Nitpick comments (17)
packages/schema/src/types/hooks.ts (1)
260-276: New compiler hooks look correct.Signatures match the intended “mutate arrays in-place” pattern for extending scan targets. Consider clarifying in JSDoc that
filesare absolute paths and the hook is called after resolving directories but before scanning/transforms.docs/3.api/6.advanced/1.hooks.md (1)
65-67: Tweak wording forcompiler:filestiming.To avoid ambiguity, consider: “Called after resolving files from the configured directories (absolute paths), before compiler transforms; extend the list here.”
packages/kit/src/compiler.ts (2)
7-13: Name‑dedupe when registering scan plugins.Pushing blindly can create duplicates. Replace by
nameif it already exists.Apply:
export function addCompilerScanPlugin (plugin: ScanPlugin) { const nuxt = useNuxt() nuxt.options.compiler ||= {} nuxt.options.compiler.plugins ||= [] - nuxt.options.compiler.plugins.push(plugin) - return plugin + const list = nuxt.options.compiler.plugins + const i = list.findIndex(p => p.name === plugin.name) + if (i === -1) list.push(plugin) + else list[i] = plugin + return plugin }
15-19: Support async plugin factories and explicit return types.Improve DX by allowing async factories and by declaring return types explicitly.
Apply:
-export function createCompilerScanPlugin (plugin: ScanPlugin): ScanPlugin -export function createCompilerScanPlugin (factory: () => ScanPlugin): ScanPlugin -export function createCompilerScanPlugin (arg: ScanPlugin | (() => ScanPlugin)): ScanPlugin { - return typeof arg === 'function' ? arg() : arg -} +export function createCompilerScanPlugin (plugin: ScanPlugin): ScanPlugin +export function createCompilerScanPlugin (factory: () => ScanPlugin | Promise<ScanPlugin>): Promise<ScanPlugin> +export function createCompilerScanPlugin (arg: ScanPlugin | (() => ScanPlugin | Promise<ScanPlugin>)): ScanPlugin | Promise<ScanPlugin> { + return typeof arg === 'function' ? arg() : arg +}If you prefer sync only, keep the current implementation but add the explicit return type for clarity:
-export function createCompilerScanPlugin (arg: ScanPlugin | (() => ScanPlugin)): ScanPlugin { +export function createCompilerScanPlugin (arg: ScanPlugin | (() => ScanPlugin)): ScanPlugin { return typeof arg === 'function' ? arg() : arg }packages/nuxt/src/compiler/utils.ts (1)
39-47: Consider explicit final return for clarity.The function correctly handles both string and RegExp matchers, but the implicit
return falseat the end (line 46) could be made explicit for better readability, especially since other branches explicitly return.Apply this diff for clarity:
export function matchWithStringOrRegex (value: string, matcher: string | RegExp) { if (typeof matcher === 'string') { return value === matcher } else if (matcher instanceof RegExp) { return matcher.test(value) + } else { + return false } - - return false }packages/nuxt/src/compiler/module.ts (1)
149-151: Harden file path resolution for non-existent entries.Some entries are module-inserted and may not exist; use a resilient resolve with explicit type and fallback to original.
- const filePaths = await Promise.all(_filePaths.map(filePath => resolvePath(filePath))) + const filePaths = await Promise.all( + _filePaths.map(fp => resolvePath(fp, { type: 'file', fallbackToOriginal: true })) + )packages/nuxt/src/compiler/plugins/keyed-function-factories.ts (1)
110-111: Dynamic regex from variable input — low risk here, but cap size.Names are internal and escaped, so ReDoS is unlikely. As a safeguard, cap the union length (e.g. slice to a reasonable number) or short‑circuit to a trivial regex if the set is empty.
const names = [...localFactoryNames].slice(0, 500) // guardrail const LOCAL_FACTORY_NAMES_RE = new RegExp(`\\b(${names.map(f => escapeRE(f)).join('|')})\\b`)Also applies to: 253-256, 334-336
packages/nuxt/test/compiler.test.ts (2)
22-34: Stabilise dev-branch test: mockimport.meta.devreliably.
vi.stubGlobal('__TEST_DEV__', true)won’t affectimport.meta.dev. Use Vite define, or wrap the check in a helper you can mock in tests (e.g.isDev()), then assert.
192-200: Avoid brittle assertions tied to parser internals.Asserting the exact node count (
7) is fragile across parser updates. Prefer presence checks only.- expect(nodes.length).toBe(7) + expect(nodes.length).toBeGreaterThan(0)packages/nuxt/src/compiler/parse-utils.ts (1)
114-161: Solid static call parsing; optional chaining can be extended later.Implementation is correct and covered by tests. The TODO to broaden optional chaining support can be deferred.
Also applies to: 211-235
packages/nuxt/src/compiler/plugins/keyed-functions.ts (4)
136-139: Avoid repeated I/O: cache auto-imports across transforms.getAutoImports() is awaited for every file; cache its promise in the plugin closure.
Apply:
export const KeyedFunctionsPlugin = (options: KeyedFunctionsOptions) => createUnplugin(() => { + let cachedAutoImportsPromise: Promise<Import[]> | null = null @@ - const autoImports = await options.getAutoImports() + const autoImports = await (cachedAutoImportsPromise ||= options.getAutoImports())
101-103: Large dynamic RegExp unions: keep bounded or switch to Set.CODE_INCLUDE_RE builds a union from potentially many identifiers. It’s build-time, but consider guarding size or pre-checking with a Set before AST work for better worst‑case behaviour.
229-257: Optional chaining calls currently not handled.parseStaticFunctionCall has a TODO for optional chaining; calls like ns?.useX?.() won’t be keyed. Either extend parser to handle ChainExpression + MemberExpression or document the limitation.
383-392: Trailing comments between args and ) can break comma detection.endsWithComma ignores block/line comments. Rare, but adding a light scanner to skip over comments before checking the last non‑whitespace token would make insertion more robust.
packages/nuxt/src/app/composables/asyncData.ts (3)
229-242: Factory options precedence/merging: confirm intended semantics.Current logic:
- Object defaults: usage opts override factory defaults (shallow copy).
- Function defaults: factory return overrides usage opts (replace).
This is shallow; nested hooks/interceptors won’t merge. Given discussion about merging handlers, please confirm this behaviour. If deep-defaults are desired, consider defu or a merge strategy per key.
Also applies to: 253-259
371-404: On key change, abort the old in-flight request to free resources.When the key changes, the old container is unregistered but its request continues. It won’t update the new key, but it still consumes network/CPU. Abort it before unregistering.
Apply:
const unsubKeyWatcher = watch(key, (newKey, oldKey) => { if ((newKey || oldKey) && newKey !== oldKey) { keyChanging = true @@ - // Now it's safe to drop the old container. - if (oldKey) { + // Abort any in-flight request for the old key, then drop the old container. + if (oldKey) { + try { + nuxtApp._asyncData[oldKey]?._abortController?.abort( + new DOMException('AsyncData aborted due to key change', 'AbortError') + ) + } catch {} unregister(oldKey) }
431-462: Internal generics vs return type cast.asyncReturn uses Ref, while the exposed type is AsyncData<PickFrom<DataT, PickKeys> | DefaultT, …>. It’s fine because of overloads and the cast, but aligning the internal generic to DataT | DefaultT would improve IDE hints and reduce casts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/schema/package.jsonis excluded by!**/package.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (34)
docs/3.api/6.advanced/1.hooks.md(1 hunks)packages/kit/src/compiler.ts(1 hunks)packages/kit/src/index.ts(1 hunks)packages/nuxt/build.config.ts(1 hunks)packages/nuxt/src/app/composables/asyncData.ts(3 hunks)packages/nuxt/src/app/composables/fetch.ts(2 hunks)packages/nuxt/src/app/config.ts(1 hunks)packages/nuxt/src/compiler/index.ts(1 hunks)packages/nuxt/src/compiler/module.ts(1 hunks)packages/nuxt/src/compiler/parse-utils.ts(1 hunks)packages/nuxt/src/compiler/plugins/keyed-function-factories.ts(1 hunks)packages/nuxt/src/compiler/plugins/keyed-functions.ts(1 hunks)packages/nuxt/src/compiler/runtime/index.ts(1 hunks)packages/nuxt/src/compiler/utils.ts(1 hunks)packages/nuxt/src/components/module.ts(3 hunks)packages/nuxt/src/core/nuxt.ts(3 hunks)packages/nuxt/src/core/plugins/composable-keys.ts(0 hunks)packages/nuxt/src/core/utils/plugins.ts(0 hunks)packages/nuxt/src/imports/presets.ts(1 hunks)packages/nuxt/src/utils.ts(2 hunks)packages/nuxt/test/compiler.test.ts(1 hunks)packages/nuxt/test/composable-keys.test.ts(0 hunks)packages/nuxt/test/keyed-function-factories.test.ts(1 hunks)packages/nuxt/test/keyed-functions.test.ts(1 hunks)packages/nuxt/test/load-nuxt.test.ts(1 hunks)packages/schema/src/config/build.ts(1 hunks)packages/schema/src/index.ts(1 hunks)packages/schema/src/types/compiler.ts(1 hunks)packages/schema/src/types/components.ts(3 hunks)packages/schema/src/types/hooks.ts(2 hunks)packages/schema/src/types/nuxt.ts(2 hunks)packages/schema/src/types/schema.ts(3 hunks)packages/schema/src/utils/definition.ts(1 hunks)test/fixtures/basic-types/app/compiler-types.ts(1 hunks)
💤 Files with no reviewable changes (3)
- packages/nuxt/src/core/utils/plugins.ts
- packages/nuxt/src/core/plugins/composable-keys.ts
- packages/nuxt/test/composable-keys.test.ts
🧰 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/load-nuxt.test.tspackages/kit/src/compiler.tspackages/nuxt/src/imports/presets.tstest/fixtures/basic-types/app/compiler-types.tspackages/schema/src/types/hooks.tspackages/nuxt/src/compiler/runtime/index.tspackages/nuxt/test/keyed-functions.test.tspackages/nuxt/src/app/config.tspackages/nuxt/src/compiler/index.tspackages/nuxt/test/keyed-function-factories.test.tspackages/kit/src/index.tspackages/schema/src/types/schema.tspackages/nuxt/src/compiler/plugins/keyed-function-factories.tspackages/schema/src/types/nuxt.tspackages/schema/src/config/build.tspackages/nuxt/src/compiler/utils.tspackages/nuxt/src/components/module.tspackages/nuxt/src/compiler/module.tspackages/nuxt/build.config.tspackages/nuxt/src/core/nuxt.tspackages/schema/src/index.tspackages/schema/src/types/compiler.tspackages/schema/src/utils/definition.tspackages/nuxt/src/utils.tspackages/nuxt/src/app/composables/fetch.tspackages/nuxt/src/compiler/plugins/keyed-functions.tspackages/nuxt/src/compiler/parse-utils.tspackages/nuxt/test/compiler.test.tspackages/schema/src/types/components.tspackages/nuxt/src/app/composables/asyncData.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/load-nuxt.test.tspackages/nuxt/test/keyed-functions.test.tspackages/nuxt/test/keyed-function-factories.test.tspackages/nuxt/test/compiler.test.ts
🧠 Learnings (1)
📚 Learning: 2024-11-05T15:22:54.759Z
Learnt from: GalacticHypernova
PR: nuxt/nuxt#26468
File: packages/nuxt/src/components/plugins/loader.ts:24-24
Timestamp: 2024-11-05T15:22:54.759Z
Learning: In `packages/nuxt/src/components/plugins/loader.ts`, the references to `resolve` and `distDir` are legacy code from before Nuxt used the new unplugin VFS and will be removed.
Applied to files:
packages/nuxt/src/components/module.ts
🧬 Code graph analysis (19)
packages/kit/src/compiler.ts (1)
packages/schema/src/types/nuxt.ts (1)
ScanPlugin(116-135)
test/fixtures/basic-types/app/compiler-types.ts (1)
packages/nuxt/src/compiler/runtime/index.ts (1)
defineKeyedFunctionFactory(17-31)
packages/schema/src/types/hooks.ts (1)
packages/schema/src/types/compiler.ts (1)
CompilerScanDir(62-84)
packages/nuxt/src/compiler/runtime/index.ts (1)
packages/nuxt/src/app/composables/fetch.ts (1)
factory(92-235)
packages/nuxt/test/keyed-functions.test.ts (2)
packages/schema/src/types/compiler.ts (1)
KeyedFunction(4-33)packages/nuxt/src/compiler/plugins/keyed-functions.ts (1)
KeyedFunctionsPlugin(52-408)
packages/nuxt/test/keyed-function-factories.test.ts (5)
packages/schema/src/types/nuxt.ts (1)
Nuxt(158-185)packages/nuxt/src/app/config.ts (1)
DeepPartial(9-9)packages/schema/src/types/compiler.ts (1)
KeyedFunctionFactory(35-42)packages/nuxt/src/compiler/plugins/keyed-function-factories.ts (2)
KeyedFunctionFactoriesScanPlugin(243-309)KeyedFunctionFactoriesPlugin(326-417)packages/nuxt/src/compiler/utils.ts (1)
createScanPluginContext(10-37)
packages/schema/src/types/schema.ts (1)
packages/schema/src/types/compiler.ts (3)
NuxtCompilerOptions(44-60)KeyedFunction(4-33)KeyedFunctionFactory(35-42)
packages/nuxt/src/compiler/plugins/keyed-function-factories.ts (4)
packages/nuxt/src/compiler/parse-utils.ts (3)
FunctionCallMetadata(79-104)parseStaticFunctionCall(114-235)processImports(15-77)packages/schema/src/types/compiler.ts (2)
KeyedFunctionFactory(35-42)KeyedFunction(4-33)packages/nuxt/src/utils.ts (4)
logger(52-52)stripExtension(34-36)isJavascriptExtension(44-47)JS_EXTENSIONS(49-49)packages/nuxt/src/compiler/plugins/keyed-functions.ts (1)
shouldTransformFile(38-47)
packages/schema/src/types/nuxt.ts (1)
packages/schema/src/utils/definition.ts (2)
Awaitable(34-34)MaybeArray(35-35)
packages/nuxt/src/compiler/utils.ts (1)
packages/schema/src/types/nuxt.ts (1)
ScanPlugin(116-135)
packages/nuxt/src/components/module.ts (1)
packages/nuxt/src/utils.ts (2)
isDirectorySync(14-16)DECLARATION_EXTENSIONS(50-50)
packages/nuxt/src/compiler/module.ts (6)
packages/schema/src/types/compiler.ts (2)
NuxtCompilerOptions(44-60)CompilerScanDir(62-84)packages/kit/src/compiler.ts (1)
addCompilerScanPlugin(7-13)packages/kit/src/resolve.ts (2)
resolvePath(52-63)resolveFiles(280-288)packages/nuxt/src/utils.ts (5)
isDirectorySync(14-16)logger(52-52)normalizeExtension(30-32)DECLARATION_EXTENSIONS(50-50)toArray(6-8)packages/schema/src/types/nuxt.ts (2)
ScanPlugin(116-135)ScanPluginFilter(141-141)packages/nuxt/src/compiler/utils.ts (2)
createScanPluginContext(10-37)matchWithStringOrRegex(39-47)
packages/schema/src/types/compiler.ts (2)
packages/schema/src/types/nuxt.ts (1)
ScanPlugin(116-135)packages/schema/src/utils/definition.ts (1)
JavascriptExtension(37-37)
packages/nuxt/src/app/composables/fetch.ts (3)
packages/nuxt/src/compiler/runtime/index.ts (1)
defineKeyedFunctionFactory(17-31)packages/nuxt/src/app/composables/asyncData.ts (2)
AsyncData(137-137)useAsyncData(468-468)packages/nuxt/src/app/composables/ssr.ts (1)
useRequestFetch(45-50)
packages/nuxt/src/compiler/plugins/keyed-functions.ts (3)
packages/schema/src/types/compiler.ts (1)
KeyedFunction(4-33)packages/nuxt/src/utils.ts (3)
stripExtension(34-36)logger(52-52)isWhitespace(38-42)packages/nuxt/src/compiler/parse-utils.ts (4)
processImports(15-77)parseStaticExportIdentifiers(261-361)FunctionCallMetadata(79-104)parseStaticFunctionCall(114-235)
packages/nuxt/src/compiler/parse-utils.ts (1)
packages/nuxt/src/utils.ts (1)
stripExtension(34-36)
packages/nuxt/test/compiler.test.ts (3)
packages/nuxt/src/compiler/runtime/index.ts (1)
defineKeyedFunctionFactory(17-31)packages/nuxt/src/compiler/utils.ts (1)
createScanPluginContext(10-37)packages/nuxt/src/compiler/parse-utils.ts (4)
FunctionCallMetadata(79-104)parseStaticFunctionCall(114-235)ExportMetadata(237-240)parseStaticExportIdentifiers(261-361)
packages/schema/src/types/components.ts (2)
packages/schema/src/types/compiler.ts (1)
CompilerScanDir(62-84)packages/schema/src/utils/definition.ts (2)
AugmentProperty(44-50)VueExtension(38-38)
packages/nuxt/src/app/composables/asyncData.ts (4)
packages/nuxt/src/app/composables/error.ts (1)
NuxtError(15-17)packages/nuxt/src/app/nuxt.ts (1)
useNuxtApp(549-562)packages/nuxt/src/app/utils.ts (1)
getUserCaller(26-42)packages/nuxt/src/app/components/client-only.ts (1)
clientOnlySymbol(8-8)
🪛 ast-grep (0.39.6)
packages/nuxt/src/compiler/plugins/keyed-function-factories.ts
[warning] 109-109: 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(\\b(${[...localFactoryNames].map(f => escapeRE(f)).join('|')})\\b)
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)
[warning] 253-253: 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(\\b(${options.factories.map(f => escapeRE(f.name)).join('|')})\\b)
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)
[warning] 334-334: 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(\\b(${options.factories.map(f => escapeRE(f.name)).join('|')})\\b)
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)
packages/nuxt/src/compiler/plugins/keyed-functions.ts
[warning] 43-43: 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(\\.(${extensions.map(e => escapeRE(e)).join('|')})$)
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)
[warning] 101-101: 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(\\b(${[...namesToSourcesToFunctionMeta.keys(), ...defaultExportSources].map(f => escapeRE(f)).join('|')})\\b)
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)
[warning] 226-226: 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(\\b(${[...possibleLocalFunctionNames].map(f => escapeRE(f)).join('|')})\\b)
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)
🔇 Additional comments (31)
packages/nuxt/src/utils.ts (2)
1-1: LGTM!The synchronous directory check implementation is clean and follows a safe pattern with appropriate error handling.
Also applies to: 14-16
34-50: LGTM!The utility functions and constants are well-implemented. The regex patterns are correct, and the type handling is appropriate.
packages/nuxt/src/components/module.ts (2)
1-1: LGTM!The import refactoring consolidates filesystem utilities into a shared module, improving code reuse and maintainability.
Also applies to: 7-7
97-97: LGTM!The synchronous directory check simplifies the code whilst maintaining safe error handling. The performance impact is negligible for single stat calls.
packages/nuxt/test/load-nuxt.test.ts (1)
135-135: LGTM! Snapshot update reflects compiler module integration.The test snapshot correctly includes the new "nuxt:compiler" module in the expected module list, consistent with the compiler subsystem introduced in this PR.
packages/nuxt/build.config.ts (1)
18-18: LGTM! Compiler runtime directory correctly added.The addition of 'compiler' to the runtime directories ensures that compiler runtime artifacts are properly generated and included in the build output, aligning with the new compiler subsystem.
packages/nuxt/src/app/config.ts (1)
9-9: LGTM! Type export improves public API surface.Exporting
DeepPartialallows consumers to reference this utility type directly, which is beneficial since it's already used in the publicupdateAppConfigAPI. This aligns with the PR's goal of expanding public type surfaces for factory-based configurations.packages/schema/src/config/build.ts (2)
80-88: LGTM! Source field addition enhances composable tracking.Adding the
sourcefield to each keyed composable entry provides the module location, which improves tooling capabilities for the compiler subsystem to track and transform composable usage across the codebase.
90-99: Verification complete:argumentLength: 3is correct.The returned
useFetchcomposable accepts a maximum of three parameters:request(required),arg1(optional), andarg2(optional), as shown in the third overload implementation. The configuration value is accurate.packages/nuxt/src/imports/presets.ts (1)
53-53: LGTM! Factory function added to auto-imports.Adding
createUseFetchto the auto-import presets makes the factory function available throughout the application, consistent with the PR's goal of exposing factory-based composable creation.packages/kit/src/index.ts (1)
35-35: LGTM! New compiler scan plugin APIs exported.The new exports
addCompilerScanPluginandcreateCompilerScanPluginextend the kit's public API surface to support compiler scan plugin registration, following the established naming conventions for kit utilities.Please ensure these new public APIs are properly documented in the relevant documentation files (e.g., docs/3.api/5.kit/ or similar) so that module authors can understand how to use them.
packages/nuxt/src/compiler/index.ts (1)
1-2: LGTM! Clean public API surface for compiler runtime.This barrel export file provides a focused public API for the compiler module, exposing
defineKeyedFunctionFactory(the main factory creation utility) and theObjectFactorytype. The minimal surface is appropriate for a new module.packages/nuxt/src/core/nuxt.ts (2)
32-32: LGTM! Compiler module import added.The import statement correctly adds the compiler module to the core Nuxt initialization flow.
800-800: LGTM! Compiler module registered in core modules.The compiler module is correctly added to the core modules list in
loadNuxt, ensuring it's loaded as part of the Nuxt initialization sequence alongside other essential modules.test/fixtures/basic-types/app/compiler-types.ts (2)
18-23: Types preserved — good.Callable, parameters, and return types are retained by
defineKeyedFunctionFactory.
35-41: The review comment is incorrect and recommends deprecated APIs.Based on the latest Vitest documentation:
.toMatchTypeOfis deprecated; the modern replacement is.toExtend(both check if a type extends/is compatible with another)..toMatchObjectTypeis valid and recommended for strict object-type matching.- The review suggests changing both line 39 and line 40 to use
toMatchTypeOf, which is a deprecated matcher.The current code:
- Line 39 uses
toEqualTypeOf(exact equality check) – acceptable for this context.- Line 40 uses
toMatchObjectType(strict object matching) – correct and valid per current API guidance.The suggested changes in the review comment should be disregarded. If a change were desired on line 39 to check assignability instead of exact equality, the modern approach would be to use
.toExtend()rather than the deprecated.toMatchTypeOf(). However,toEqualTypeOfis also valid and appropriate for generic parameter type assertions. The code as written is correct.Likely an incorrect or invalid review comment.
packages/schema/src/index.ts (1)
4-4: Public types re-export — good surface.The expanded export surface matches the new compiler and scan plugin APIs.
Also applies to: 10-10
packages/schema/src/types/schema.ts (2)
61-65: Newcompilerconfig — OK.Type is clear and consistent with added hooks/plugins.
505-519: Review comment is incorrect.The type definitions in
packages/schema/src/types/compiler.tsshow thatKeyedFunction['source']has always been typed asstring(required, not optional). The codebase contains no evidence ofsourceever acceptingRegExpor being optional. All usage patterns across fixtures, tests, and default configurations use string sources exclusively (e.g.,'#app/composables/once','~/composables/useKey'). No backwards compatibility concerns exist here.Likely an incorrect or invalid review comment.
packages/nuxt/test/keyed-function-factories.test.ts (2)
1-535: LGTM! Comprehensive test coverage for keyed function factories.The test suite thoroughly covers:
- Named and default exports
- Import variations (with/without extensions, renamed, namespaced, bracket notation)
- Auto-imports and non-matching sources
- Shadowing and scope handling
- Edge cases like type-only imports and chain expressions
The test structure is clear and assertions validate expected behaviour effectively.
537-830: LGTM! Thorough coverage of transformation plugin behaviour.The transform plugin tests validate:
- Placeholder macro replacement with
__nuxt_factoryaccessor- Various import shapes and renaming scenarios
- Optional chaining and bracket notation handling
- Proper handling of non-matching sources, dynamic imports, and type-only imports
- Edge cases like local shadowing, default exports, and already-transformed code
The snapshot-based assertions ensure transformation correctness across diverse scenarios.
packages/nuxt/src/compiler/utils.ts (1)
10-37: LGTM! Efficient lazy initialization pattern for shared parsing.The
createScanPluginContextfunction correctly implements a lazy initialization pattern that shares expensive parse operations (AST parsing and import analysis) across multiple scan plugins operating on the same file. The conditional logic ensures parsing occurs only once per file.packages/nuxt/test/keyed-functions.test.ts (2)
1-799: LGTM! Comprehensive test coverage for keyed functions plugin.The test suite thoroughly validates:
- Hash injection for various call patterns
- Import handling (default, named, namespace, mixed, renamed)
- Shadowing scenarios (function, variable, parameter)
- Export handling in source files
- Edge cases like trailing commas, multi-line parameters, type-only imports
- Special handling for template literals, dynamic access, and spread arguments
The tests are well-organized and assertions confirm expected transformation behaviour.
800-939: LGTM! Thorough validation of core keyed functions behaviour.The core keyed functions tests validate special handling for:
useState: Detecting string literal keys to avoid redundant hash injectionuseFetch/useLazyFetch: Always injecting keys regardless of argumentsuseAsyncData/useLazyAsyncData: Detecting existing keys in first or last positionThis ensures the plugin correctly handles Nuxt's built-in composables with their specific key patterns.
packages/schema/src/utils/definition.ts (1)
34-50: LGTM! Well-defined type utilities with clear documentation.The new type exports are appropriate:
Awaitable<T>andMaybeArray<T>are common utility typesJavascriptExtensionandVueExtensionprovide type-safe extension literalsAugmentProperty<T, K, V>includes helpful JSDoc explaining its behaviour with array propertiesThe types are correctly implemented and will be useful across the compiler subsystem.
packages/schema/src/types/compiler.ts (1)
1-84: LGTM! Well-documented compiler type definitions.The new interfaces provide a clear public API for the Nuxt compiler subsystem:
KeyedFunctionandKeyedFunctionFactoryare well-documented with helpful examplesNuxtCompilerOptionsprovides clear configuration surfaceCompilerScanDiroffers flexible directory scanning configuration with appropriate defaultsThe JSDoc comments effectively explain the purpose and usage of each interface, particularly the transformation examples in
KeyedFunction.packages/nuxt/src/app/composables/fetch.ts (2)
86-236: LGTM! Factory pattern correctly implements flexible default options.The
createUseFetchfactory correctly:
- Accepts either static options or a dynamic getter function
- Merges factory options with call-site options appropriately (static options are overridable, getter results replace)
- Preserves all existing
useFetchfunctionality including SSR support, key generation, and request handling- Maintains proper type safety throughout the implementation
The factory pattern enables users to create custom fetch composables with pre-defined defaults whilst retaining full type inference.
238-244: Factory unwrapping pattern is verbose but necessary.The type casting to access
__nuxt_factoryis verbose ((createUseFetch as unknown as { __nuxt_factory: typeof createUseFetch }).__nuxt_factory()), but this is the expected pattern for unwrapping the placeholder macro at runtime. The non-enumerable property ensures it doesn't interfere with normal usage whilst providing access to the underlying factory.As noted in the past review comments, consider whether there's a more ergonomic API for module authors who want to manually define keyed function entries without scanning. The current approach requires understanding the
__nuxt_factoryaccessor pattern.Based on learnings
packages/schema/src/types/nuxt.ts (1)
77-142: LGTM! Well-designed ScanPlugin system with shared parsing optimization.The ScanPlugin system is well-architected:
ScanPluginHandlerThisContextprovides sharedwalkParsedandgetParsedStaticImportsutilities to avoid redundant parsing across plugins scanning the same fileScanPluginHandlerContextexposes necessary file and Nuxt instance contextScanPluginFilteroffers flexible filtering via functions or patterns with include/exclude support- The
afterScanhook enables plugins to register build transformations after scanning completesThe type definitions are clear, well-documented, and enable efficient plugin composition.
packages/nuxt/src/compiler/runtime/index.ts (1)
17-25: Critical: Early return prevents error from being thrown in development.The condition on line 19 returns early when
!import.meta.dev(i.e., in production), which means the error on lines 20-24 is only thrown in production. This is backwards—the error should be thrown in development to help developers catch misuse early, not in production where it could break the application.Apply this diff to fix the logic:
const placeholder = function () { - if (!import.meta.dev) { return } + if (import.meta.dev) { throw new Error( `[nuxt:compiler] \`${factory.name}\` is a compiler macro that is only usable inside ` + // TODO: add link to docs about this factory function 'the directories scanned by the Nuxt compiler as an exported function and imported statically. Learn more: `https://nuxt.com/docs/TODO`', ) + } }Likely an incorrect or invalid review comment.
packages/schema/src/types/components.ts (1)
44-45: Type surface looks consistent with compiler scan config.Extending from
CompilerScanDirand augmentingextensionsfor Vue is sensible and keeps components options aligned.Also applies to: 84-85
| async handler (code, _id) { | ||
| const { 0: script = code, index: codeIndex = 0 } = code.match(SCRIPT_RE) || { 0: code, index: 0 } | ||
| const id = stripExtension(_id) | ||
|
|
||
| const { directImports, namespaces } = processImports(findStaticImports(script).map(i => parseStaticImport(i)), options.alias) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
SFC imports: scan all script blocks (avoid missing imports).
Only the first <script> is scanned for imports, so SFCs with both <script setup> and <script> can miss mappings, breaking key injection. Scan the whole file for imports and normalise id to drop query before stripExtension.
Apply:
- const { 0: script = code, index: codeIndex = 0 } = code.match(SCRIPT_RE) || { 0: code, index: 0 }
- const id = stripExtension(_id)
+ // Scan entire file so both <script setup> and <script> imports are seen
+ const script = code
+ const codeIndex = 0
+ // Normalise id for comparisons with keyed function sources
+ const id = stripExtension(_id.split('?')[0])🤖 Prompt for AI Agents
In packages/nuxt/src/compiler/plugins/keyed-functions.ts around lines 112 to
117, the import scan is only run on the first matched <script> block which
misses imports in other script blocks (e.g., <script setup> + <script>), and the
id is not normalized to remove query strings before stripExtension; change the
import scanning to run findStaticImports over the entire original file content
(use the incoming code variable directly) and feed those imports into
processImports(parseStaticImport(...)), and normalize the id by removing any
query string (split on '?' and take the first part) before calling
stripExtension so the extension stripping works correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/schema/src/types/schema.ts (1)
61-64: Consider expanding the documentation.The documentation for the new
compilerconfiguration field is quite minimal. Consider adding more context about what the compiler does and when users might need to configure it, such as:/** - * Configure the Nuxt compiler. + * Configure the Nuxt compiler for build-time transformations. + * + * This enables scanning of directories and running compiler plugins that transform + * your code during the build process (e.g., for keyed composable injection). + * + * @see [NuxtCompilerOptions](packages/schema/src/types/compiler.ts) */ compiler: NuxtCompilerOptionspackages/schema/src/types/components.ts (1)
79-79: Clever type augmentation for Vue extensions.The
ComponentsDirinterface correctly augments theextensionsproperty to includeVueExtensionwhilst preserving the base types and optional modifier fromCompilerScanDir. The type composition usingOmit,Pick, andAugmentPropertyachieves type-safe property augmentation, though it's worth noting the complexity for future maintainers.If type complexity becomes an maintenance concern, consider documenting the augmentation pattern or defining
extensionsdirectly onComponentsDirwith an explicit union type.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/schema/package.jsonis excluded by!**/package.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (8)
docs/3.api/6.advanced/1.hooks.md(1 hunks)packages/kit/src/index.ts(1 hunks)packages/nuxt/build.config.ts(1 hunks)packages/nuxt/src/app/config.ts(1 hunks)packages/nuxt/src/core/nuxt.ts(3 hunks)packages/schema/src/config/build.ts(1 hunks)packages/schema/src/types/components.ts(3 hunks)packages/schema/src/types/schema.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/nuxt/src/app/config.ts
- packages/nuxt/build.config.ts
- packages/nuxt/src/core/nuxt.ts
- packages/kit/src/index.ts
🧰 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/schema/src/types/components.tspackages/schema/src/types/schema.tspackages/schema/src/config/build.ts
🧬 Code graph analysis (2)
packages/schema/src/types/components.ts (2)
packages/schema/src/types/compiler.ts (1)
CompilerScanDir(62-84)packages/schema/src/utils/definition.ts (2)
AugmentProperty(44-50)VueExtension(38-38)
packages/schema/src/types/schema.ts (1)
packages/schema/src/types/compiler.ts (3)
NuxtCompilerOptions(44-60)KeyedFunction(4-33)KeyedFunctionFactory(35-42)
🔇 Additional comments (4)
docs/3.api/6.advanced/1.hooks.md (1)
65-67: Documentation changes are accurate and well-integrated.The three new compiler hooks are properly placed in the "Nuxt Hooks (build time)" section with clear descriptions and consistent formatting. The arguments and descriptions align correctly with the PR objectives and compiler system design.
As an optional enhancement (for later), the descriptions could reference related documentation or configuration (e.g., linking to
addCompilerScanPluginkit utilities mentioned in the PR), but this is not necessary for this release.packages/schema/src/types/schema.ts (1)
42-42: LGTM: Import statement is correct.The new types are properly imported and used in the schema below.
packages/schema/src/types/components.ts (2)
1-2: LGTM! Required imports for type composition.The imports correctly bring in the base scanning interface and utility types needed for the refactored component directory types.
42-44: Good refactoring to share scanning properties.The
ScanDirinterface correctly inherits common scanning properties (path,pattern,ignore) fromCompilerScanDirwhilst excludingextensionsfor component-specific handling. The TODO appropriately notes that component-related properties (prefix, pathPrefix, prefetch, etc.) should eventually move fromScanDirtoComponentsDirfor better separation of concerns.
| /** | ||
| * Functions to inject a key for. | ||
| * | ||
| * As long as the number of arguments passed to the function is lower than `argumentLength`, an additional magic string will be injected that can be used to deduplicate requests between server and client. You will need to take steps to handle this additional key. | ||
| * The key will be unique based on the location of the function being invoked within the file. | ||
| * | ||
| */ | ||
| keyedComposables: KeyedFunction[] | ||
| /** | ||
| * Factories for functions that should be registered for automatic key injection. | ||
| * | ||
| * @see keyedComposables | ||
| */ | ||
| keyedComposableFactories: KeyedFunctionFactory[] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Breaking change: keyedComposables type has stricter requirements.
The type change from the inline definition to KeyedFunction[] introduces breaking changes:
sourceis now required (was optional:source?: string | RegExp)sourceno longer acceptsRegExp(now onlystring)
Users who have configured keyedComposables in their nuxt.config may encounter type errors if they:
- Omitted the
sourceproperty (it was optional before) - Used a
RegExppattern forsource
Ensure this breaking change is:
- Documented in migration guides
- Mentioned in release notes
- Covered by the PR description's issue resolutions
If backward compatibility is needed, consider:
- Making
sourceoptional inKeyedFunctionwith a default value - Supporting
RegExppatterns via a union type
🤖 Prompt for AI Agents
In packages/schema/src/types/schema.ts around lines 505 to 518, the change to
use KeyedFunction[] made the source property required and string-only, which
breaks existing configs that omitted source or used RegExp; update the
KeyedFunction type to make source optional and accept both string and RegExp
(source?: string | RegExp) and ensure any code that reads source handles
undefined by using a sensible default or fallback behavior, then add a short
migration note/release note entry documenting the compatibility change and the
recommended configuration update for users still passing RegExp patterns.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/schema/package.jsonis excluded by!**/package.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (2)
packages/nuxt/src/app/composables/asyncData.ts(3 hunks)packages/nuxt/src/app/composables/fetch.ts(2 hunks)
🧰 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/app/composables/fetch.tspackages/nuxt/src/app/composables/asyncData.ts
🧬 Code graph analysis (2)
packages/nuxt/src/app/composables/fetch.ts (3)
packages/nuxt/src/compiler/runtime/index.ts (1)
defineKeyedFunctionFactory(17-31)packages/nuxt/src/app/composables/asyncData.ts (5)
KeysOf(33-39)AsyncData(137-137)PickFrom(23-31)AsyncDataOptions(49-107)useAsyncData(468-468)packages/nuxt/src/app/composables/ssr.ts (1)
useRequestFetch(45-50)
packages/nuxt/src/app/composables/asyncData.ts (4)
packages/nuxt/src/app/composables/error.ts (1)
NuxtError(15-17)packages/nuxt/src/app/nuxt.ts (1)
useNuxtApp(549-562)packages/nuxt/src/app/utils.ts (1)
getUserCaller(26-42)packages/nuxt/src/app/components/client-only.ts (1)
clientOnlySymbol(8-8)
⏰ 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). (2)
- GitHub Check: build
- GitHub Check: code
🔇 Additional comments (6)
packages/nuxt/src/app/composables/fetch.ts (3)
11-11: LGTM!The import is necessary for the factory pattern and is correctly placed.
238-244: Factory pattern verified—approved as intentional design.The
__nuxt_factoryproperty access pattern is correct and thoroughly verified. It is defined as a non-enumerable getter in the compiler runtime (packages/nuxt/src/compiler/runtime/index.ts:27) and the KeyedFunctionFactoriesPlugin transforms factory calls during compilation. Extensive tests in keyed-function-factories.test.ts confirm the pattern transforms correctly. The implementation matches the established design used in asyncData.ts and is consistent throughout the codebase.The
@ts-expect-errorfor the private_functionNameproperty is appropriate for the development-only debugging property.
161-180: Verify whether hook merging is an actual requirement for this factory pattern.The concern about hook handlers being overridden rather than merged is technically accurate given the spread operator logic, however the codebase evidence does not confirm this is a documented requirement or user expectation:
- No test cases demonstrate expected hook merging behaviour.
- PR objectives regarding hook merging could not be verified in git history.
- No hook-merging utilities exist in the codebase.
- The current implementation uses standard object spread semantics.
Confirm whether users should be able to compose hooks from both factory and call-site options. If this is a requirement, the current spread-based merging approach would need refactoring to handle hook composition (e.g., array-based handlers or explicit merge logic).
The
as anycast on line 161 is a pragmatic choice for managing TypeScript's complexity with these generic types and does not require changes.packages/nuxt/src/app/composables/asyncData.ts (3)
264-288: Excellent developer experience with incompatible options detection.The warning system effectively catches common mistakes where the same key is reused with incompatible options. The use of
getUserCaller()provides helpful source location context, and the comprehensive checks cover all critical option differences.
371-411: Sophisticated key migration logic with proper synchronisation.The key watching implementation correctly:
- Migrates data from old to new key containers
- Manages reference counting to avoid memory leaks
- Prevents race conditions using the
keyChangingguard- Triggers fetches appropriately based on prior state
The use of
flush: 'sync'combined withqueuePostFlushCbensures the guard is released after all synchronous updates complete, preventing double execution from the params watcher.
605-768: Rename fromcreateAsyncDatatobuildAsyncDatais appropriate.The internal implementation has been correctly renamed to
buildAsyncDatato avoid confusion with the new public factorycreateAsyncData. The core logic for data fetching, caching, hydration, and error handling is preserved, ensuring backward compatibility whilst enabling the factory pattern.
useFetch and useAsyncDatauseFetch and useAsyncData
useFetch and useAsyncDatauseFetch and useAsyncData
|
@cernymatej it's ready ? |
# Conflicts: # docs/3.api/6.advanced/1.hooks.md # packages/kit/src/index.ts # packages/nuxt/src/app/composables/asyncData.ts # packages/nuxt/src/app/composables/fetch.ts # packages/nuxt/src/components/module.ts # packages/nuxt/src/core/nuxt.ts # packages/nuxt/src/utils.ts # packages/nuxt/test/compiler.test.ts # packages/nuxt/test/keyed-functions.test.ts # packages/schema/src/config/build.ts # packages/schema/src/index.ts # packages/schema/src/types/compiler.ts # packages/schema/src/types/hooks.ts # packages/schema/src/types/nuxt.ts # packages/schema/src/types/schema.ts
|
No dependency changes detected. Learn more about Socket for GitHub. 👍 No dependency changes detected in pull request |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/kit/src/compiler.ts (1)
2-2: Remove.tsextension from import path.Importing with a
.tsextension breaks standard TypeScript and module resolution. Use the extensionless path instead.🔎 Proposed fix
-import { useNuxt } from './context.ts' +import { useNuxt } from './context'packages/nuxt/src/compiler/module.ts (1)
204-204: HMR support remains unaddressed.The TODO comment flags that HMR is tricky because scan plugins modify
nuxt.options.optimization.keyedComposablesduring theafterScanhook. This was previously noted and remains an open question.
🧹 Nitpick comments (6)
packages/kit/src/compiler.ts (1)
15-19: Consider documenting the factory pattern use case.The implementation correctly handles both direct plugins and factory functions. However, since the factory is eagerly evaluated (not lazy), it would be helpful to document when and why developers should use the factory pattern versus passing a plugin directly.
💡 Optional: Add JSDoc clarifying factory usage
+/** + * Creates a compiler scan plugin. + * + * @param plugin - A ScanPlugin or factory function returning a ScanPlugin + * @returns The resolved ScanPlugin + * + * @example + * // Direct plugin + * createCompilerScanPlugin({ name: 'my-plugin', scan: ... }) + * + * // Factory for dynamic plugin creation + * createCompilerScanPlugin(() => ({ + * name: 'my-plugin', + * scan: (context) => { ... } + * })) + */ export function createCompilerScanPlugin (plugin: ScanPlugin): ScanPlugin export function createCompilerScanPlugin (factory: () => ScanPlugin): ScanPlugin export function createCompilerScanPlugin (arg: ScanPlugin | (() => ScanPlugin)): ScanPlugin { return typeof arg === 'function' ? arg() : arg }packages/nuxt/src/core/utils/parse-utils.ts (1)
7-27: Consider adding JSDoc to document the new parameter.The
aliasparameter lacks documentation explaining its purpose and expected format. Adding JSDoc would improve maintainability.🔎 Suggested documentation
+/** + * Processes static imports to extract direct imports and namespace imports. + * @param imports Array of parsed static imports + * @param alias Alias mapping (e.g., from Nuxt's alias configuration) to resolve import specifiers + * @returns Object containing directImports and namespaces maps + */ export function processImports (imports: ParsedStaticImport[], alias: Record<string, string>) {packages/nuxt/src/compiler/module.ts (1)
64-202: Consider breaking downrunScanPluginsinto smaller helper functions.The function is 138 lines and handles multiple responsibilities: directory collection, normalisation, file resolution, plugin deduplication, and scanning. Whilst the sequential flow is logical, extracting discrete phases (e.g.,
collectScanDirs,normaliseDirectories,deduplicatePlugins) would improve maintainability and testability.Example refactoring approach
Extract logical sections into focused helpers:
async function collectScanDirs(layers, options, resolvePath) { // Lines 78-103 logic } async function normaliseDirectories(scanDirs, resolvePath) { // Lines 107-136 logic } async function resolveFilesFromDirs(scanDirs, resolveFiles) { // Lines 140-150 logic }Then compose them in
runScanPlugins.packages/nuxt/src/compiler/plugins/keyed-function-factories.ts (3)
253-253: Track support for default import.The TODO notes that default imports won't have the factory name in the pattern, which could lead to missed matches. Consider opening an issue to track this enhancement if not already planned.
Do you want me to open a new issue to track support for default imports in factory detection?
355-355: Track auto-imports support.The TODO notes that auto-imports are not yet supported in the transform plugin, which could lead to missed factory rewrites when factories are auto-imported.
Do you want me to open a new issue to track auto-imports support in the factory rewriting plugin?
359-384: Optional chaining detection lacks whitespace handling.The code checks
code[node.end] === '?'to detect optional chaining, which fails if whitespace exists between the identifier and the?.operator. Whilst test coverage confirms the transformation works for standard formatted code (without extra whitespace), the approach is fragile. The existing TODO comment correctly identifies this limitation. Consider using AST node properties or a more defensive pattern to handle edge cases with unexpected whitespace formatting.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/schema/package.jsonis excluded by!**/package.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (9)
packages/kit/src/compiler.tspackages/nuxt/src/compiler/index.tspackages/nuxt/src/compiler/module.tspackages/nuxt/src/compiler/plugins/keyed-function-factories.tspackages/nuxt/src/compiler/plugins/keyed-functions.tspackages/nuxt/src/core/nuxt.tspackages/nuxt/src/core/utils/parse-utils.tspackages/schema/src/config/build.tspackages/schema/src/types/hooks.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/nuxt/src/compiler/index.ts
- packages/schema/src/config/build.ts
- packages/nuxt/src/compiler/plugins/keyed-functions.ts
- packages/nuxt/src/core/nuxt.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/kit/src/compiler.tspackages/nuxt/src/core/utils/parse-utils.tspackages/nuxt/src/compiler/module.tspackages/nuxt/src/compiler/plugins/keyed-function-factories.tspackages/schema/src/types/hooks.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx,vue}: Use clear, descriptive variable and function names
Add comments only to explain complex logic or non-obvious implementations
Keep functions focused and manageable (generally under 50 lines), and extract complex logic into separate domain-specific files
Remove code that is not used or needed
Use error handling patterns consistently
Files:
packages/kit/src/compiler.tspackages/nuxt/src/core/utils/parse-utils.tspackages/nuxt/src/compiler/module.tspackages/nuxt/src/compiler/plugins/keyed-function-factories.tspackages/schema/src/types/hooks.ts
🧠 Learnings (12)
📚 Learning: 2024-11-28T21:22:40.496Z
Learnt from: GalacticHypernova
Repo: nuxt/nuxt PR: 29661
File: packages/kit/src/template.ts:227-229
Timestamp: 2024-11-28T21:22:40.496Z
Learning: In `packages/kit/src/template.ts`, when updating the `EXTENSION_RE` regular expression for TypeScript configuration, avoid using patterns like `(\.\w+)+$` as they can result in catastrophic backtracking.
Applied to files:
packages/kit/src/compiler.tspackages/nuxt/src/compiler/module.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/*.{ts,tsx,vue} : Follow standard TypeScript conventions and best practices
Applied to files:
packages/kit/src/compiler.tspackages/nuxt/src/compiler/module.tspackages/schema/src/types/hooks.ts
📚 Learning: 2024-12-12T12:36:34.871Z
Learnt from: huang-julien
Repo: nuxt/nuxt PR: 29366
File: packages/nuxt/src/app/components/nuxt-root.vue:16-19
Timestamp: 2024-12-12T12:36:34.871Z
Learning: In `packages/nuxt/src/app/components/nuxt-root.vue`, when optimizing bundle size by conditionally importing components based on route metadata, prefer using inline conditional imports like:
```js
const IsolatedPage = route?.meta?.isolate ? defineAsyncComponent(() => import('#build/isolated-page.mjs')) : null
```
instead of wrapping the import in a computed property or importing the component unconditionally.
Applied to files:
packages/kit/src/compiler.tspackages/nuxt/src/compiler/plugins/keyed-function-factories.tspackages/schema/src/types/hooks.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/*.{ts,tsx,js,jsx,vue} : Remove code that is not used or needed
Applied to files:
packages/kit/src/compiler.tspackages/nuxt/src/compiler/module.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/*.vue : Use `<script setup lang="ts">` and the composition API when creating Vue components
Applied to files:
packages/kit/src/compiler.tspackages/schema/src/types/hooks.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/*.{ts,tsx,js,jsx,vue} : Use error handling patterns consistently
Applied to files:
packages/kit/src/compiler.tspackages/nuxt/src/compiler/module.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/*.{ts,tsx,js,jsx,vue} : Use clear, descriptive variable and function names
Applied to files:
packages/kit/src/compiler.ts
📚 Learning: 2024-11-05T15:22:54.759Z
Learnt from: GalacticHypernova
Repo: nuxt/nuxt PR: 26468
File: packages/nuxt/src/components/plugins/loader.ts:24-24
Timestamp: 2024-11-05T15:22:54.759Z
Learning: In `packages/nuxt/src/components/plugins/loader.ts`, the references to `resolve` and `distDir` are legacy code from before Nuxt used the new unplugin VFS and will be removed.
Applied to files:
packages/kit/src/compiler.tspackages/nuxt/src/compiler/module.tspackages/nuxt/src/compiler/plugins/keyed-function-factories.tspackages/schema/src/types/hooks.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/*.{ts,tsx,js,jsx,vue} : Keep functions focused and manageable (generally under 50 lines), and extract complex logic into separate domain-specific files
Applied to files:
packages/kit/src/compiler.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/*.{ts,tsx,js,jsx,vue} : Add comments only to explain complex logic or non-obvious implementations
Applied to files:
packages/kit/src/compiler.ts
📚 Learning: 2024-11-11T10:45:48.403Z
Learnt from: Tofandel
Repo: nuxt/nuxt PR: 29876
File: eslint.config.mjs:76-76
Timestamp: 2024-11-11T10:45:48.403Z
Learning: In ESLint, changing `varsIgnorePattern` to flag unused variables is safe because unused variables are locally declared and their removal doesn't affect the code. However, changing `argsIgnorePattern` might have unintended side effects since unused function arguments could be necessary for correct function signatures or implicit typing.
Applied to files:
packages/nuxt/src/compiler/module.ts
📚 Learning: 2025-04-18T18:33:41.753Z
Learnt from: TheAlexLichter
Repo: nuxt/nuxt PR: 31812
File: packages/nuxt/src/components/plugins/islands-transform.ts:202-202
Timestamp: 2025-04-18T18:33:41.753Z
Learning: In Nuxt, using `rolldownVersion` (not `rollupVersion`) is intentional when detecting if rolldown-vite is being used, even though TypeScript may show an error because the property isn't in standard type definitions yet.
Applied to files:
packages/schema/src/types/hooks.ts
🧬 Code graph analysis (4)
packages/kit/src/compiler.ts (3)
packages/kit/src/index.ts (3)
addCompilerScanPlugin(37-37)useNuxt(30-30)createCompilerScanPlugin(37-37)packages/schema/src/index.ts (1)
ScanPlugin(12-12)packages/schema/src/types/nuxt.ts (1)
ScanPlugin(116-135)
packages/nuxt/src/core/utils/parse-utils.ts (1)
packages/nuxt/src/utils.ts (1)
stripExtension(34-36)
packages/nuxt/src/compiler/module.ts (9)
packages/kit/src/index.ts (6)
defineNuxtModule(2-2)addCompilerScanPlugin(37-37)addBuildPlugin(22-22)resolvePath(38-38)logger(42-42)resolveFiles(38-38)packages/kit/src/compiler.ts (1)
addCompilerScanPlugin(7-13)packages/nuxt/src/compiler/plugins/keyed-function-factories.ts (2)
KeyedFunctionFactoriesScanPlugin(243-309)KeyedFunctionFactoriesPlugin(326-417)packages/kit/src/resolve.ts (2)
resolvePath(53-64)resolveFiles(281-289)packages/nuxt/src/compiler/plugins/keyed-functions.ts (1)
KeyedFunctionsPlugin(47-403)packages/nuxt/src/utils.ts (5)
isDirectorySync(14-16)logger(52-52)normalizeExtension(30-32)DECLARATION_EXTENSIONS(50-50)toArray(6-8)packages/schema/src/types/nuxt.ts (2)
ScanPlugin(116-135)ScanPluginFilter(141-141)packages/nuxt/src/compiler/utils.ts (2)
createScanPluginContext(10-37)matchWithStringOrRegex(39-47)packages/kit/src/utils.ts (1)
toArray(2-4)
packages/schema/src/types/hooks.ts (2)
packages/schema/src/index.ts (2)
HookResult(8-8)CompilerScanDir(4-4)packages/schema/src/types/compiler.ts (1)
CompilerScanDir(62-84)
🪛 ast-grep (0.40.3)
packages/nuxt/src/compiler/plugins/keyed-function-factories.ts
[warning] 109-109: 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(\\b(${[...localFactoryNames].map(f => escapeRE(f)).join('|')})\\b)
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)
[warning] 253-253: 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(\\b(${options.factories.map(f => escapeRE(f.name)).join('|')})\\b)
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)
[warning] 334-334: 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(\\b(${options.factories.map(f => escapeRE(f.name)).join('|')})\\b)
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)
⏰ 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). (1)
- GitHub Check: code
🔇 Additional comments (15)
packages/schema/src/types/hooks.ts (1)
14-14: LGTM!The import is correctly added to support the
compiler:dirshook type annotation on line 276.packages/kit/src/compiler.ts (1)
7-13: Clean implementation of plugin registration.The use of nullish coalescing assignment ensures the nested structure exists before pushing the plugin. The pattern is appropriate for a kit utility that modifies Nuxt configuration.
packages/nuxt/src/core/utils/parse-utils.ts (2)
27-27: LGTM!The alias parameter is correctly passed to
resolveAliasto enable alias-aware import resolution. The implementation properly threads the new parameter through the resolution pipeline.
7-7: All call sites have been properly updated with the required alias parameter.Verification confirms that both usages of
processImportscorrectly pass the alias parameter:
packages/nuxt/src/compiler/plugins/keyed-function-factories.ts:102passes thealiasfunction parameterpackages/nuxt/src/compiler/plugins/keyed-functions.ts:111passesoptions.alias(typed asRecord<string, string>perKeyedFunctionsOptions)The
resolveAliasfunction from@nuxt/kitaccepts the alias mapping as an optional second parameter (alias?: Record<string, string>), making it fully compatible with the implementation.packages/nuxt/src/compiler/module.ts (3)
1-12: LGTM!The imports and constants are well-organised. All necessary dependencies are properly imported, types are correctly referenced from
@nuxt/schema, and theruntimeDirconstant follows established Nuxt patterns.
14-62: LGTM!The module definition and setup are well-structured. The unimport context capture enables downstream auto-import resolution, plugin registration is properly sequenced, and the factory function is correctly exposed through the imports system.
213-218: LGTM!The
matchFilterhelper correctly handles both function-based and pattern-based filters with include/exclude semantics, leveraging thetoArrayandmatchWithStringOrRegexutilities appropriately.packages/nuxt/src/compiler/plugins/keyed-function-factories.ts (8)
1-26: LGTM!The imports are well-organised, covering all necessary parsing, transformation, and utility dependencies. The
ParsedKeyedFunctionFactoryinterface is clearly structured with descriptive field names.
32-84: LGTM!The
parseKeyedFunctionFactoryfunction correctly handles both named and default export patterns. The use ofScopeTrackerto resolve export specifiers and theprocessVariableDeclaratorhelper keep the logic maintainable.
94-110: Static analysis warning is a false positive.Line 109 constructs a regex from factory names, but
escapeREproperly escapes all special regex characters, and the resulting pattern\b(name1|name2|...)\bis safe with bounded alternation. No ReDoS risk exists here.
113-226: LGTM!The factory processor correctly handles path resolution, import mapping (both direct and namespace), and validates factory sources with appropriate logging. The logic properly skips unresolved factories and prevents descending into invalid subtrees.
243-261: Static analysis warning is a false positive.Line 254 constructs a regex from factory names using
escapeRE, which properly escapes special characters. The pattern is safe and poses no ReDoS risk.
262-308: LGTM!The scan logic efficiently walks the AST with performance optimisations (skipping irrelevant subtrees), correctly uses
ScopeTrackerto resolve declarations, and properly registers discovered factories in theafterScanhook.
326-344: Static analysis warning is a false positive.Line 335 uses
escapeREto escape factory names before constructing the regex pattern, making it safe from ReDoS attacks.
386-414: LGTM!The transform handler correctly parses and walks the AST, processes factories with proper scoping, and generates sourcemaps when configured. The MagicString usage is appropriate for code rewriting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/nuxt/test/compiler.test.ts (1)
502-770: LGTM!The
parseExporttest suite provides thorough coverage of export parsing functionality across various export patterns. The use of inline snapshots makes the expected output clear and easy to verify. Edge cases such as default export handling, filtering behaviour, and type export exclusion are all properly tested.Consider splitting this test file into multiple smaller files (e.g.,
compiler-factory.test.ts,compiler-scan.test.ts,compiler-parse-calls.test.ts,compiler-parse-exports.test.ts) to improve maintainability. At 770 lines, it's becoming quite large, and smaller files would make it easier to locate specific tests and run targeted test suites.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/nuxt/src/core/nuxt.tspackages/nuxt/test/compiler.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/nuxt/src/core/nuxt.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/nuxt/test/compiler.test.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx,vue}: Use clear, descriptive variable and function names
Add comments only to explain complex logic or non-obvious implementations
Keep functions focused and manageable (generally under 50 lines), and extract complex logic into separate domain-specific files
Remove code that is not used or needed
Use error handling patterns consistently
Files:
packages/nuxt/test/compiler.test.ts
**/*.{test,spec}.{ts,tsx,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Write unit tests for core functionality using
vitest
Files:
packages/nuxt/test/compiler.test.ts
🧠 Learnings (1)
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/*.{test,spec}.{ts,tsx,js} : Write unit tests for core functionality using `vitest`
Applied to files:
packages/nuxt/test/compiler.test.ts
🧬 Code graph analysis (1)
packages/nuxt/test/compiler.test.ts (2)
packages/nuxt/src/compiler/runtime/index.ts (1)
defineKeyedFunctionFactory(17-31)packages/nuxt/src/compiler/utils.ts (1)
createScanPluginContext(10-37)
⏰ 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). (16)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite-env-api, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite-env-api, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, default, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, rspack, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, rspack, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, webpack, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite-env-api, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-on, js, lts/-1)
- GitHub Check: typecheck (windows-latest, bundler)
- GitHub Check: typecheck (ubuntu-latest, bundler)
- GitHub Check: release-pkg-pr-new
- GitHub Check: test-size
- GitHub Check: test-benchmark
- GitHub Check: code
🔇 Additional comments (6)
packages/nuxt/test/compiler.test.ts (6)
1-11: LGTM!The imports are well-organised and appropriate for testing the Nuxt compiler functionality. All necessary testing utilities and modules are properly imported.
13-15: LGTM!The
transformFactoryhelper cleanly simulates the compiler's transformation step by accessing the__nuxt_factoryproperty. The implementation is simple and appropriately typed.
93-99: LGTM!The
importModuleshelper properly supports the mocking strategy used in the subsequent tests by dynamically importing the required modules. The implementation is clean and serves its purpose well.
101-201: LGTM!The
createScanPluginContexttest suite provides excellent coverage with both unit tests (using mocked dependencies) and integration-style tests. The mocked tests properly verify the parse result caching behaviour, whilst the real AST walking test validates scope tracking and node traversal. This dual approach ensures both the optimisation logic and the actual functionality are well-tested.
203-500: LGTM!The
parseFunctionCalltest suite is exceptionally comprehensive, covering a wide variety of JavaScript and TypeScript syntax patterns. The tests properly validate both parseable cases (simple calls, member expressions, optional chaining, TypeScript constructs) and non-parseable cases (dynamic expressions, computed properties, etc.). The helper functions are well-designed and the test organisation is clear.
22-34: No changes needed. The test correctly mocksimport.meta.devthrough Vitest's Vite configuration, which replacesimport.meta.devwithglobalThis.__TEST_DEV__during test execution. The subsequentvi.stubGlobal('__TEST_DEV__', true)properly simulates dev mode behaviour.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/schema/src/config/build.ts (1)
89-103: Verify factory name and argument length.The past review identified that the factory should be named
createAsyncDatarather thancreateUseAsyncData(line 97), and this was marked as addressed. However, the current code still showscreateUseAsyncData. Additionally, theargumentLengthvalue of 3 for both factories should be verified against their actual signatures, as factory functions may have different arity than the composables they produce.Run the following script to verify the actual exported factory names and their signatures:
#!/bin/bash # Description: Verify factory exports and signatures from asyncData.ts and fetch.ts echo "=== Factory exports from asyncData.ts ===" rg -n --type=ts -A2 'export (const|function) create.*AsyncData' packages/nuxt/src/app/composables/asyncData.ts echo -e "\n=== Factory exports from fetch.ts ===" rg -n --type=ts -A2 'export (const|function) create.*Fetch' packages/nuxt/src/app/composables/fetch.ts echo -e "\n=== Factory function signatures ===" ast-grep --pattern $'export const $FACTORY = defineKeyedFunctionFactory($$$)'
🧹 Nitpick comments (1)
packages/nuxt/src/compiler/module.ts (1)
198-199: Consider implementing HMR support for scan plugins.The TODO comment highlights a known limitation: scan plugins modify
nuxt.options.optimization.keyedComposables, which currently requires a Nuxt restart to reflect changes. Consider implementing file-watching with incremental re-scanning to improve developer experience.Would you like me to generate a design proposal for HMR support that handles incremental scanning and updates to the keyed composables configuration?
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (6)
packages/nuxt/src/compiler/module.tspackages/nuxt/src/compiler/plugins/keyed-function-factories.tspackages/nuxt/src/core/nuxt.tspackages/nuxt/test/compiler.test.tspackages/schema/src/config/build.tspackages/schema/src/types/hooks.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/nuxt/test/compiler.test.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/schema/src/types/hooks.tspackages/nuxt/src/compiler/module.tspackages/schema/src/config/build.tspackages/nuxt/src/core/nuxt.tspackages/nuxt/src/compiler/plugins/keyed-function-factories.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx,vue}: Use clear, descriptive variable and function names
Add comments only to explain complex logic or non-obvious implementations
Keep functions focused and manageable (generally under 50 lines), and extract complex logic into separate domain-specific files
Remove code that is not used or needed
Use error handling patterns consistently
Files:
packages/schema/src/types/hooks.tspackages/nuxt/src/compiler/module.tspackages/schema/src/config/build.tspackages/nuxt/src/core/nuxt.tspackages/nuxt/src/compiler/plugins/keyed-function-factories.ts
🧠 Learnings (7)
📚 Learning: 2024-11-28T21:22:40.496Z
Learnt from: GalacticHypernova
Repo: nuxt/nuxt PR: 29661
File: packages/kit/src/template.ts:227-229
Timestamp: 2024-11-28T21:22:40.496Z
Learning: In `packages/kit/src/template.ts`, when updating the `EXTENSION_RE` regular expression for TypeScript configuration, avoid using patterns like `(\.\w+)+$` as they can result in catastrophic backtracking.
Applied to files:
packages/nuxt/src/compiler/module.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/*.{ts,tsx,js,jsx,vue} : Use error handling patterns consistently
Applied to files:
packages/nuxt/src/compiler/module.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/*.{ts,tsx,js,jsx,vue} : Remove code that is not used or needed
Applied to files:
packages/nuxt/src/compiler/module.tspackages/nuxt/src/core/nuxt.ts
📚 Learning: 2024-11-11T10:45:48.403Z
Learnt from: Tofandel
Repo: nuxt/nuxt PR: 29876
File: eslint.config.mjs:76-76
Timestamp: 2024-11-11T10:45:48.403Z
Learning: In ESLint, changing `varsIgnorePattern` to flag unused variables is safe because unused variables are locally declared and their removal doesn't affect the code. However, changing `argsIgnorePattern` might have unintended side effects since unused function arguments could be necessary for correct function signatures or implicit typing.
Applied to files:
packages/nuxt/src/compiler/module.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/*.{ts,tsx,vue} : Follow standard TypeScript conventions and best practices
Applied to files:
packages/nuxt/src/compiler/module.ts
📚 Learning: 2024-11-05T15:22:54.759Z
Learnt from: GalacticHypernova
Repo: nuxt/nuxt PR: 26468
File: packages/nuxt/src/components/plugins/loader.ts:24-24
Timestamp: 2024-11-05T15:22:54.759Z
Learning: In `packages/nuxt/src/components/plugins/loader.ts`, the references to `resolve` and `distDir` are legacy code from before Nuxt used the new unplugin VFS and will be removed.
Applied to files:
packages/nuxt/src/compiler/module.tspackages/nuxt/src/core/nuxt.tspackages/nuxt/src/compiler/plugins/keyed-function-factories.ts
📚 Learning: 2024-12-12T12:36:34.871Z
Learnt from: huang-julien
Repo: nuxt/nuxt PR: 29366
File: packages/nuxt/src/app/components/nuxt-root.vue:16-19
Timestamp: 2024-12-12T12:36:34.871Z
Learning: In `packages/nuxt/src/app/components/nuxt-root.vue`, when optimizing bundle size by conditionally importing components based on route metadata, prefer using inline conditional imports like:
```js
const IsolatedPage = route?.meta?.isolate ? defineAsyncComponent(() => import('#build/isolated-page.mjs')) : null
```
instead of wrapping the import in a computed property or importing the component unconditionally.
Applied to files:
packages/schema/src/config/build.tspackages/nuxt/src/core/nuxt.tspackages/nuxt/src/compiler/plugins/keyed-function-factories.ts
🧬 Code graph analysis (1)
packages/schema/src/types/hooks.ts (1)
packages/schema/src/index.ts (1)
HookResult(8-8)
🪛 ast-grep (0.40.3)
packages/nuxt/src/compiler/plugins/keyed-function-factories.ts
[warning] 109-109: 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(\\b(${[...localFactoryNames].map(f => escapeRE(f)).join('|')})\\b)
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)
[warning] 253-253: 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(\\b(${options.factories.map(f => escapeRE(f.name)).join('|')})\\b)
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)
[warning] 334-334: 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(\\b(${options.factories.map(f => escapeRE(f.name)).join('|')})\\b)
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)
⏰ 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). (4)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite-env-api, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, rspack, async, manifest-on, json, lts/-1)
- GitHub Check: test-size
- GitHub Check: code
🔇 Additional comments (2)
packages/nuxt/src/compiler/plugins/keyed-function-factories.ts (2)
109-109: RegExp construction from variable input is mitigated byescapeRE.Static analysis tools flagged these lines as potentially vulnerable to ReDoS attacks. However, the code uses
escapeREto sanitise all user-provided factory names before constructing the pattern\b(name1|name2|...)\b, which prevents injection attacks and catastrophic backtracking. The pattern structure is safe.Note: This is informational. The mitigation is already in place. Based on static analysis hints, already addressed.
Also applies to: 253-253, 334-334
355-355: This review comment is based on a misunderstanding of the plugin architecture. The transform plugin intentionally passesundefinedfor auto-imports because it runs only after the scan plugin's results have been used to inject explicit imports into the source code. By the time the transform plugin executes, all factory imports—including those originally auto-imported—have been converted to explicit imports, making theautoImportsToSourcesparameter unnecessary. The TODO comment does not indicate incomplete functionality, as confirmed by test coverage showing the transform plugin correctly handles factories that were originally auto-imported but have been made explicit.Likely an incorrect or invalid review comment.
🔗 Linked issue
resolves #14736
resolves #14936
resolves #28792
resolves #28485
resolves #24157
resolves #33008
📚 Description
This PR introduces a way to create custom
useFetchanduseAsyncDatafunctions with user-defined default options.These custom instances can be configured with either static options or a dynamic getter, which can be used to access values from
runtimeConfig, for example.In the case of a getter, the user must manually handle merging with the usage options, as the returned options object will replace them. When a plain object is passed into the factory, the usage options will automatically override the options set in the factory function.
How it works
Both composables rely on automatic key injection. For this reason, it was necessary to create a mechanism to automatically register these new custom instances to
optimization.keyedComposables. This registration occurs during Nuxt initialization in a newnuxt:compilermodule, where specified directories are scanned for usages of these factory functions. Each export is processed, and the corresponding composable identifier is registered for key injection.The factory functions should be treated more like compiler macros. Because of this, there are only a few places where they can be used - as exports from files in the directories scanned by the new Nuxt compiler. To warn users about incorrect usage, the runtime version of these factory functions simply throws an error. Only when they are correctly recognized and processed during compilation are they replaced with the actual factory functions.
🔦 Nuxt compiler
This is a new core module I added to make it easier to hook into file scanning, should it be needed for something else in the future. The module ensures that files are read only once, and all its plugins share the contents (as well as the AST, if necessary).
It is possible to configure which directories should be scanned by the module, as well as which files within those directories:
It comes with
@nuxt/kitutilities:addCompilerScanPlugin(plugin: ScanPlugin)- adds a newScanPluginto the Nuxt compiler / scannercreateCompilerScanPlugin(plugin: ScanPlugin): ScanPlugin- future-proofing wrapper for creating the scan pluginsand new hooks:
💬 Additional points to discuss
- Should provided options likeonRequestcompletely override the defaults when calling the composable, or should they merge with them? We could potentially separate these two strategies under different keys in the factory configuration object. Currently, everything is grouped underdefaults, but I'm considering introducing a second key (e.g.,merge) that would merge the options. This would allow users to define anonRequesthandler when calling the composable, while still executing the one defined in the factory function. (similar issue unjs/ofetch#319)useFetchpass cookies from the server to the client (by default?). Is this something we want to tackle in this PR, or should we revisit it later? (Simplify Set-Cookie header forwarding #32306)keyoption is set #28169UseFetchOptionsextendable? This would allow users to add custom configuration keys to theiruseFetchinstances, which they could then handle in the dynamic getter version of the factory function (by applying different logic in request hooks, etc.) - related: Add aresetAfteroption touseFetchto automatically reset the state after a specified time. #28792🚧 TODO
@nuxt/kitutilities for creating factories@nuxt/kitutilities for creating keyed factoriesexport the internaluseFetchutilities likegenerateOptionSegmentsand user-friendly types for better customizationcome up with a guide for manual creation of the custom instances (without auto imports and compiler scanning)useAsyncData()