-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
fix(nuxt): add missing element/vnode props for <NuxtLink>
#33335
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
Conversation
|
@nuxt/kit
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
CodSpeed Performance ReportMerging #33335 will not alter performanceComparing Summary
|
WalkthroughThe NuxtLink component typing in packages/nuxt/src/app/components/nuxt-link.ts is updated to augment its props with VNodeProps, AllowedComponentProps, and AnchorHTMLAttributes. The exported component’s constructor type now accepts NuxtLinkProps combined with these standard attributes. The internal DefineSetupFnComponent type is adjusted to propagate the augmented prop set through its setup signature. No runtime logic changes are indicated; modifications are to TypeScript types for the component’s public and internal typings. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/nuxt/src/app/components/nuxt-link.ts
(1 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/components/nuxt-link.ts
⏰ 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: test-fixtures (windows-latest, built, rspack, async, manifest-on, json, lts/-1)
}) as unknown as (new<CustomProp extends boolean = false>(props: NuxtLinkProps<CustomProp> & VNodeProps & AllowedComponentProps & AnchorHTMLAttributes) => InstanceType<DefineSetupFnComponent< | ||
NuxtLinkProps<CustomProp> & VNodeProps & AllowedComponentProps & AnchorHTMLAttributes, | ||
[], | ||
SlotsType<NuxtLinkSlots<CustomProp>> | ||
>>) & Record<string, any> |
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.
Avoid narrowing href
/rel
types when intersecting props
Intersecting NuxtLinkProps
with AnchorHTMLAttributes
collapses the shared keys. In practice, href
gets narrowed to string
, so passing a route object (which we support and rely on above) now produces a type error. The same happens for rel
/target
, stripping the null
allowance and other specific unions we expose. Please omit the overlapping keys before merging in the anchor attributes.
- }) as unknown as (new<CustomProp extends boolean = false>(props: NuxtLinkProps<CustomProp> & VNodeProps & AllowedComponentProps & AnchorHTMLAttributes) => InstanceType<DefineSetupFnComponent<
- NuxtLinkProps<CustomProp> & VNodeProps & AllowedComponentProps & AnchorHTMLAttributes,
+ }) as unknown as (new<CustomProp extends boolean = false>(props: NuxtLinkProps<CustomProp> & VNodeProps & AllowedComponentProps & Omit<AnchorHTMLAttributes, 'href' | 'rel' | 'target'>) => InstanceType<DefineSetupFnComponent<
+ NuxtLinkProps<CustomProp> & VNodeProps & AllowedComponentProps & Omit<AnchorHTMLAttributes, 'href' | 'rel' | 'target'>,
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
}) as unknown as (new<CustomProp extends boolean = false>(props: NuxtLinkProps<CustomProp> & VNodeProps & AllowedComponentProps & AnchorHTMLAttributes) => InstanceType<DefineSetupFnComponent< | |
NuxtLinkProps<CustomProp> & VNodeProps & AllowedComponentProps & AnchorHTMLAttributes, | |
[], | |
SlotsType<NuxtLinkSlots<CustomProp>> | |
>>) & Record<string, any> | |
}) as unknown as (new<CustomProp extends boolean = false>( | |
props: NuxtLinkProps<CustomProp> & VNodeProps & AllowedComponentProps & Omit<AnchorHTMLAttributes, 'href' | 'rel' | 'target'> | |
) => InstanceType<DefineSetupFnComponent< | |
NuxtLinkProps<CustomProp> & VNodeProps & AllowedComponentProps & Omit<AnchorHTMLAttributes, 'href' | 'rel' | 'target'>, | |
[], | |
SlotsType<NuxtLinkSlots<CustomProp>> | |
>>) & Record<string, any> |
🤖 Prompt for AI Agents
In packages/nuxt/src/app/components/nuxt-link.ts around lines 529 to 533, the
intersection with AnchorHTMLAttributes collapses overlapping keys (e.g., href,
rel, target) causing href to be narrowed to string and losing null/union
options; fix by omitting the overlapping keys from the anchor attributes before
merging (use Omit<AnchorHTMLAttributes, keyof NuxtLinkProps<CustomProp>> or
explicitly Omit<AnchorHTMLAttributes, "href" | "rel" | "target">) so the
NuxtLinkProps definitions for href/rel/target are preserved when combined with
anchor attributes.
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.
In practice, href gets narrowed to string, so passing a route object ... now produces a type error.
This warning seems to be correct - i get type errors on NuxtLink when passing an object as href in v4.1.3
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.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
🔗 Linked issue
resolves #33304
📚 Description
this adds some missing props/attrs accepted by
<NuxtLink>