fix(NuxtLinkLocale): make component generic to support custom prop …#4007
fix(NuxtLinkLocale): make component generic to support custom prop …#4007houtan-rocky wants to merge 2 commits into
custom prop …#4007Conversation
…uxt-modules#3558) Convert component definition from non‑generic `defineComponent<NuxtLinkLocaleProps<boolean>>` to a generic function pattern. This allows the `custom` prop to accept `true` and correctly infers slot props, resolving the TypeScript error `Type 'true' is not assignable to type 'false'`.
Walkthrough
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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.
🧹 Nitpick comments (1)
src/runtime/components/NuxtLinkLocale.ts (1)
46-119: ⚡ Quick winConsider forwarding all slots instead of just the default slot.
Line 118 currently passes
slots.defaultto the childNuxtLinkcomponent. While this works for the default slot, a more idiomatic and future-proof approach would be to pass the entireslotsobject:return () => h(NuxtLink, getNuxtLinkProps(), slots)This pattern ensures that any named slots (current or future) are properly forwarded to
NuxtLinkwithout requiring code changes.♻️ Proposed change
- return () => h(NuxtLink, getNuxtLinkProps(), slots.default) + return () => h(NuxtLink, getNuxtLinkProps(), slots)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/runtime/components/NuxtLinkLocale.ts` around lines 46 - 119, The NuxtLinkLocaleImpl component currently forwards only the default slot to the NuxtLink component in its return statement. To ensure all named slots are properly forwarded to NuxtLink, replace `slots.default` with the entire `slots` object in the h(NuxtLink, getNuxtLinkProps(), slots.default) call. This makes the slot forwarding more idiomatic and future-proof by automatically including any named slots without requiring code changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/runtime/components/NuxtLinkLocale.ts`:
- Around line 46-119: The NuxtLinkLocaleImpl component currently forwards only
the default slot to the NuxtLink component in its return statement. To ensure
all named slots are properly forwarded to NuxtLink, replace `slots.default` with
the entire `slots` object in the h(NuxtLink, getNuxtLinkProps(), slots.default)
call. This makes the slot forwarding more idiomatic and future-proof by
automatically including any named slots without requiring code changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 60cf4b37-a42d-4927-81b3-7e9782e56f0e
📒 Files selected for processing (1)
src/runtime/components/NuxtLinkLocale.ts
…ectly Add test case in `prefix.spec.ts` that ensures `<NuxtLinkLocale custom>`: - Renders a custom element with the correct localized href - Navigates to the expected route when clicked This confirms the generic component definition works as expected and prevents regression of nuxt-modules#3558.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
specs/basic_usage.spec.ts (1)
793-795: Replace raw click with clickNavigate for consistent navigation pattern.Line 793 uses
click()+waitForURL(), but this suite usesclickNavigate()throughout for route transitions. This pattern is more reliable for navigation timing.Suggested patch
- await page.locator('`#custom-link` a').click() - await page.waitForURL(url('/en/about')) + await page.locator('`#custom-link` a').clickNavigate() expect(page.url()).toContain('/en/about')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@specs/basic_usage.spec.ts` around lines 793 - 795, The click() and waitForURL() pattern used on the `#custom-link` a locator is inconsistent with the clickNavigate() pattern used throughout the rest of the test suite. Replace the separate click() and waitForURL() calls with a single clickNavigate() call to maintain consistency with the established navigation pattern and improve reliability for navigation timing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@specs/basic_usage.spec.ts`:
- Around line 793-795: The click() and waitForURL() pattern used on the
`#custom-link` a locator is inconsistent with the clickNavigate() pattern used
throughout the rest of the test suite. Replace the separate click() and
waitForURL() calls with a single clickNavigate() call to maintain consistency
with the established navigation pattern and improve reliability for navigation
timing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 19fc4603-ed21-4578-99e3-9984db8e5fd4
📒 Files selected for processing (2)
specs/basic_usage.spec.tsspecs/fixtures/basic/pages/index.vue
✅ Files skipped from review due to trivial changes (1)
- specs/fixtures/basic/pages/index.vue
🔗 Linked issue
#3558
📚 Description
Convert component definition from non‑generic
defineComponent<NuxtLinkLocaleProps<boolean>>to a generic function pattern. This allows thecustomprop to accepttrueand correctly infers slot props, resolving the TypeScript errorType 'true' is not assignable to type 'false'.Summary by CodeRabbit
hrefoutput and that clicking the link routes to the correct localized page.