fix(lazy-load): use NuxtComponent name for auto-imports#235
fix(lazy-load): use NuxtComponent name for auto-imports#235huang-julien merged 3 commits intomainfrom
Conversation
commit: |
📝 WalkthroughWalkthroughThe lazy-load plugin now integrates with Nuxt: it calls Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Actionable comments posted: 8
🧹 Nitpick comments (3)
src/plugins/lazy-load.ts (1)
19-19: Non-null assertion onnuxt.apps.defaultmay throw.
nuxt.apps.default!.componentsuses a non-null assertion. Ifnuxt.apps.defaultisundefined(e.g., during early initialization or in edge cases), this would throw aTypeErrorat plugin instantiation time with no clear error message. Consider adding a guard or fallback:-let nuxtComponents: Component[] = nuxt.apps.default!.components +let nuxtComponents: Component[] = nuxt.apps.default?.components ?? []🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugins/lazy-load.ts` at line 19, The non-null assertion on nuxt.apps.default in the assignment to nuxtComponents can throw if apps.default is undefined; update the initialization in lazy-load.ts to guard against that by using optional chaining and a safe fallback (e.g., const nuxtComponents = nuxt.apps.default?.components ?? []), or explicitly check for nuxt.apps.default and throw a clear error message before accessing .components; locate the assignment to nuxtComponents and replace the direct non-null access to avoid a runtime TypeError.test/unit/hydration/lazy-hydration-plugin.test.ts (2)
30-38: Existing tests calltransformwithoutthiscontext, unlike the newpascalNametest.Tests at Lines 34, 47, etc. call
transform(code, id)directly (nothis), but the newpascalNametest at Line 160 usest.call(unpluginCtx, ...). If the transform handler accessesthis(e.g.,this.error,this.warn), the existing tests would silently succeed withthisbeingundefined. Consider calling all tests consistently via.call(unpluginCtx, ...)to match real runtime behavior, or at minimum verify the existing tests aren't masking issues.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/hydration/lazy-hydration-plugin.test.ts` around lines 30 - 38, The tests call transform(...) directly which leaves the plugin transform handler's this undefined; update tests (including the default imports tests that reference transform and the new pascalName test) to invoke the transform function with the plugin context via transform.call(unpluginCtx, code, '/src/Parent.vue') (or the appropriate id) so that handlers like this.error/this.warn behave as in real runtime; search for usages of transform in the lazy-hydration tests and replace direct calls with .call(unpluginCtx, ...) for consistency.
132-164: Consider testing that_sfc_mainmetadata also usespascalNamewhen matched.This test only verifies the
__wrapImportedComponentwrapper usespascalName. Given the inconsistency I flagged insrc/plugins/lazy-load.tsbetween wrapper creation and_sfc_main/defineComponentmetadata paths, it would be valuable to also assert thatcomponentNamein the_sfc_mainmetadata (anddefineComponentsetup injection) reflects thepascalNamewhen a matching nuxt component exists.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/hydration/lazy-hydration-plugin.test.ts` around lines 132 - 164, Update the test to also assert that the generated component metadata uses the pascalName from nuxtComponents: after obtaining result.code from calling PluginWithComponents.vite() → t (the transform handler), add assertions that the _sfc_main metadata and any injected defineComponent/setup name reflect 'MyWidgetPascal' (look for symbols like _sfc_main, componentName metadata assignment, or defineComponent name injection in result.code) so both the __wrapImportedComponent wrapper and the _sfc_main/defineComponent metadata use the nuxtComponents pascalName.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/plugins/lazy-load.ts`:
- Around line 130-133: Typo in the inline comment: change "// See nuxt loadeer
plugin" to "// See nuxt loader plugin" near the conditional that checks
imp.name.startsWith('__nuxt') in the lazy-load logic; update the comment
adjacent to the return that references parse(imp.source).name and
normalizePath(id) so it reads "loader" instead of "loadeer".
- Around line 8-9: Add a blank line after the last import statements to satisfy
ESLint: insert an empty line after the import lines that import useNuxt and
Component (symbols: useNuxt, Component) so there is a separation between imports
and the following non-import code.
- Line 3: The import statement in src/plugins/lazy-load.ts has a trailing comma
("import { parse, resolve, } from 'node:path'") causing the
`@stylistic/comma-dangle` failure; fix it by removing the trailing comma so the
import reads "import { parse, resolve } from 'node:path'" and then run the
linter/CI to verify; locate the import line referencing parse and resolve in
this file to apply the change.
- Around line 86-93: The wrapper creation and metadata emission resolve
component names differently which causes mismatches: replace the inline lookups
with a single helper (e.g., resolveComponentName) that accepts an import
descriptor (imp) and returns the correct pascal name by consulting
nuxtComponents (falling back to parse(imp.source).name); then use this helper
wherever names are computed—specifically in the wrapperStatements mapping that
calls __wrapImportedComponent, in the _sfc_main wrapping logic (the block that
currently uses parse(imp.source).name), and in the defineComponent setup
injection—so all references (originalName, the name passed into
__wrapImportedComponent, and the __nuxt-prefixed metadata) derive from the same
resolved value.
- Around line 15-22: The createUnplugin callback for LazyLoadHintPlugin is
padded by a leading blank line which violates the no-block-padding ESLint rule;
remove the empty line immediately after the opening brace of the createUnplugin
callback so the first statement (const nuxt = useNuxt()) directly follows the
opening brace in the LazyLoadHintPlugin definition and ensure similar blocks
(e.g., where nuxt, nuxtComponents and the nuxt.hook('components:extend', ...)
are declared) have no extra blank line padding.
In `@test/unit/hydration/lazy-hydration-plugin.test.ts`:
- Line 2: The import line currently imports beforeEach but it is unused and
causing a lint error; update the import statement (the one importing describe,
it, expect, vi, beforeEach) to remove beforeEach so it only imports describe,
it, expect, and vi, ensuring no other references to beforeEach remain in the
file.
- Around line 21-29: The object literal unpluginCtx has inconsistent spacing
around colons causing ESLint failures; update the properties addWatchFile,
emitFile, getWatchFiles, parse, getNativeBuildContext, error, and warn so each
key uses a single space after the colon (e.g., addWatchFile: vi.fn()) and remove
the extra space after the warn key so it reads warn: vi.fn(), ensuring
consistent formatting across the unpluginCtx object.
- Around line 132-164: Add the two lint fixes in the test block: add a trailing
comma after the object property preload: false in the mocked component entry
(the object inside useNuxt mock) and add spacing around the union operator in
the transform handler type cast so it reads 'code' | 'id' (the cast on
(p.transform as ObjectHook<HookFnMap['transform'], 'code'|'id'>).handler
referencing PluginWithComponents / LazyLoadHintPlugin.vite()).
---
Nitpick comments:
In `@src/plugins/lazy-load.ts`:
- Line 19: The non-null assertion on nuxt.apps.default in the assignment to
nuxtComponents can throw if apps.default is undefined; update the initialization
in lazy-load.ts to guard against that by using optional chaining and a safe
fallback (e.g., const nuxtComponents = nuxt.apps.default?.components ?? []), or
explicitly check for nuxt.apps.default and throw a clear error message before
accessing .components; locate the assignment to nuxtComponents and replace the
direct non-null access to avoid a runtime TypeError.
In `@test/unit/hydration/lazy-hydration-plugin.test.ts`:
- Around line 30-38: The tests call transform(...) directly which leaves the
plugin transform handler's this undefined; update tests (including the default
imports tests that reference transform and the new pascalName test) to invoke
the transform function with the plugin context via transform.call(unpluginCtx,
code, '/src/Parent.vue') (or the appropriate id) so that handlers like
this.error/this.warn behave as in real runtime; search for usages of transform
in the lazy-hydration tests and replace direct calls with .call(unpluginCtx,
...) for consistency.
- Around line 132-164: Update the test to also assert that the generated
component metadata uses the pascalName from nuxtComponents: after obtaining
result.code from calling PluginWithComponents.vite() → t (the transform
handler), add assertions that the _sfc_main metadata and any injected
defineComponent/setup name reflect 'MyWidgetPascal' (look for symbols like
_sfc_main, componentName metadata assignment, or defineComponent name injection
in result.code) so both the __wrapImportedComponent wrapper and the
_sfc_main/defineComponent metadata use the nuxtComponents pascalName.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
test/unit/hydration/lazy-hydration-plugin.test.ts (2)
90-130:transformcalled withoutthisbinding in new tests; onlynuxtComponents matchinguses.call(unpluginCtx, ...).The inconsistency is harmless today (the handler never reads
this), but if any future change addsthis.warn()/this.error()to the handler, most tests would throw rather than fail gracefully. Consistently using.call(unpluginCtx, ...)(or.bind(unpluginCtx)once at declaration) across all tests would make the suite more resilient.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/hydration/lazy-hydration-plugin.test.ts` around lines 90 - 130, Tests call the transform handler without a this binding in several new cases; keep behavior consistent by invoking transform with the plugin context so future uses of this.warn()/this.error() don't throw. Update the tests that call transform(...) to call transform.call(unpluginCtx, ...) (or bind the context once when declaring transform) so the handler always runs with unpluginCtx as its this value; reference the transform function used in the tests and the existing use of .call(unpluginCtx, ...) in the nuxtComponents-matching tests to guide the change.
132-164: Test only validates wrapper name — missing coverage for tracking-metadata paths.The
nuxtComponents matching (pascalName)test correctly asserts that the wrapper call uses'MyWidgetPascal', but the_sfc_maintracking block (line 133 in the plugin) and thedefineComponentsetup injection (line 186) are not exercised with this mock. Because those paths bypassnuxtComponentsand useparse(source).nameinstead, a component like'MyWidgetPascal'would silently appear as'MyWidget'in the tracking payload — and no test would catch it.Consider extending the test to include a
_sfc_main/defineComponentpattern and assert that the tracking array also containscomponentName: 'MyWidgetPascal':✅ Suggested additional assertion
const code = `import MyWidget from './MyWidget.vue'\nexport default { components: { MyWidget } }` +// Also verify tracking metadata uses the same pascalName +const sfcCode = [ + `import MyWidget from './MyWidget.vue'`, + `const _sfc_main = {}`, + `export default _sfc_main`, +].join('\n') +const sfcResult = await t.call(unpluginCtx, sfcCode, '/src/Parent.vue') as unknown as { code: string } +expect(sfcResult.code).toContain(`componentName: 'MyWidgetPascal'`)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/hydration/lazy-hydration-plugin.test.ts` around lines 132 - 164, Extend the existing test for nuxtComponents matching to also exercise the _sfc_main tracking block and defineComponent injection paths: after importing PluginWithComponents and obtaining the transform handler (t) use a source string that includes a SFC default export pattern (e.g., const _sfc_main = {}; _sfc_main.render = ...; export default _sfc_main) and/or a defineComponent({...}) pattern that imports MyWidget, then run t.call(...) and add assertions that the generated tracking array (the injected tracking-metadata) includes an entry with componentName: 'MyWidgetPascal' and the correct file path './MyWidget.vue' so both wrapper name and tracking metadata are validated for nuxtComponents pascalName matching.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/plugins/lazy-load.ts`:
- Around line 86-93: Unify component name resolution by adding a
resolveComponentName helper inside the createUnplugin callback that closes over
nuxtComponents and returns the pascalName for a given import (falling back to
parse(source).name when not found and handling any __nuxt prefix logic), then
replace the inline nuxtComponents.find(...) and parse(...) usages: use
resolveComponentName when building wrapperStatements (previously around
__wrapImportedComponent(...) creation), when computing the name in the _sfc_main
tracking block, and inside injectUseLazyComponentTrackingInComponentSetup;
remove the redundant local component lookups and the now-unnecessary __nuxt
prefix guard, and update injectUseLazyComponentTrackingInComponentSetup's
signature and all call sites to accept and use resolveComponentName (or
nuxtComponents) so all three code paths share identical name resolution.
- Line 132: Comment typo: change "loadeer" to "loader" in the inline comment
within src/plugins/lazy-load.ts (the comment that reads "See nuxt loadeer
plugin"); update that comment text to "See nuxt loader plugin" and search the
file for any duplicate misspellings to correct them as well.
---
Nitpick comments:
In `@test/unit/hydration/lazy-hydration-plugin.test.ts`:
- Around line 90-130: Tests call the transform handler without a this binding in
several new cases; keep behavior consistent by invoking transform with the
plugin context so future uses of this.warn()/this.error() don't throw. Update
the tests that call transform(...) to call transform.call(unpluginCtx, ...) (or
bind the context once when declaring transform) so the handler always runs with
unpluginCtx as its this value; reference the transform function used in the
tests and the existing use of .call(unpluginCtx, ...) in the
nuxtComponents-matching tests to guide the change.
- Around line 132-164: Extend the existing test for nuxtComponents matching to
also exercise the _sfc_main tracking block and defineComponent injection paths:
after importing PluginWithComponents and obtaining the transform handler (t) use
a source string that includes a SFC default export pattern (e.g., const
_sfc_main = {}; _sfc_main.render = ...; export default _sfc_main) and/or a
defineComponent({...}) pattern that imports MyWidget, then run t.call(...) and
add assertions that the generated tracking array (the injected
tracking-metadata) includes an entry with componentName: 'MyWidgetPascal' and
the correct file path './MyWidget.vue' so both wrapper name and tracking
metadata are validated for nuxtComponents pascalName matching.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/plugins/lazy-load.ts`:
- Line 19: Replace the unsafe non-null assertion on nuxt.apps.default used to
initialize nuxtComponents: instead of using nuxt.apps.default! assume it may be
absent and initialize nuxtComponents defensively (e.g., use optional chaining
with a fallback to an empty array), and subscribe to the components:extend hook
to update or seed nuxtComponents when the authoritative components list becomes
available (reference symbols: nuxt.apps.default, nuxtComponents,
components:extend).
🔗 Linked issue
❓ Type of change
📚 Description
Nuxt's auto imports uses
__nuxt_component**as component var. This PR fixe logs and devtools hints to show the real auto-imported component('s name