fix(routerlink): allow event handlers under strictTemplates (fix #2484) #2548

hodimbokom · 2025-09-09T12:08:53Z

Description

With vueCompilerOptions.strictTemplates: true, adding any event handler to RouterLink (e.g. @focus) triggers a TS error, even though the handler works at runtime (see #2484).

Solution

Make RouterLink generic over custom to preserve anchor event/attrs when custom !== true and exclude them when custom === true.
Add Omit<AnchorHTMLAttributes, 'href'> so href stays disallowed (we derive it from to).
No runtime changes, only types.

Tests

Add dts tests to ensure:
- event handlers and anchor attrs are allowed when not custom
- href is not allowed
- anchor attrs/handlers are disallowed when custom is true
All tests pass:
- pnpm -C packages/router build && pnpm -C packages/router build:dts
- pnpm -C packages/router test:types
- pnpm -C packages/router test (coverage/unit/e2e)
Locally verified with vue-tsc --noEmit and strictTemplates: true on an SFC.

Links

Closes #2484

Also happy to adjust if maintainers prefer another approach.

Summary by CodeRabbit

New Features
- Improved TypeScript typings for RouterLink to better reflect usage modes: when not using custom, standard anchor attributes (e.g., target, rel) and native events are allowed while href is excluded; when using custom, native anchor events are restricted. No runtime changes.
Tests
- Added type-check tests to validate the updated RouterLink typings and ensure correct allowances and errors across modes.

netlify · 2025-09-09T12:09:00Z

✅ Deploy Preview for vue-router canceled.

Name	Link
🔨 Latest commit	`b0ac394`
🔍 Latest deploy log	https://app.netlify.com/projects/vue-router/deploys/68e671492b072b0008e0c0db

coderabbitai · 2025-09-09T12:09:02Z

Walkthrough

RouterLink TypeScript typings were made generic to distinguish prop shapes when custom is set or not; AnchorHTMLAttributes was imported and new internal typed aliases were added. DTS tests were expanded to verify allowed/disallowed anchor attributes and event handlers in both modes. No runtime behavior changed.

Changes

Cohort / File(s)	Summary
RouterLink typing changes `packages/router/src/RouterLink.ts`	Added `AnchorHTMLAttributes` import; introduced internal `_RouterLinkPropsTypedBase` and generic `RouterLinkPropsTyped<Custom>` types; made `_RouterLinkI` constructor generic to expose a typed `$props` surface depending on `Custom`. No runtime code changes.
DTS tests updated `packages/router/test-dts/components.test-d.tsx`	Expanded type-check tests to assert event handlers and anchor attributes are allowed when `custom` is not set, assert `href` is disallowed in that mode, and assert certain events (e.g., `onFocus`) are disallowed when `custom` is present.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I nibble on types where link-flowers grow,
Tuning props gently so event-handlers show.
Custom or not, I sort what’s allowed,
Tests hop beside me, tidy and proud.
A rabbit’s small tweak — no runtime to slow. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title precisely describes the key change of allowing event handlers on RouterLink under strictTemplates and references the related issue, succinctly conveying the primary purpose of the pull request.
Linked Issues Check	✅ Passed	The pull request updates the RouterLink type definitions to permit anchor event handlers when strictTemplates is enabled, implements a generic parameter to discriminate custom usage, omits href correctly, and adds corresponding dts tests, directly satisfying the requirements of issue #2484.
Out of Scope Changes Check	✅ Passed	All modifications are confined to RouterLink type definitions and related type tests, and there are no unrelated or extraneous code changes outside the scope of enabling event handlers under strictTemplates.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (3)

packages/router/src/RouterLink.ts (2)
1-31: Prefer type-only import for AnchorHTMLAttributes to avoid any runtime import risk

Move AnchorHTMLAttributes to a separate type-only import for consistency and to keep bundlers from attempting to load it at runtime.
-import {
+import {
   defineComponent,
   h,
   PropType,
   inject,
   computed,
   reactive,
   unref,
   VNode,
   UnwrapRef,
   VNodeProps,
   AllowedComponentProps,
   ComponentCustomProps,
   getCurrentInstance,
   watchEffect,
   // this is a workaround for https://github.com/microsoft/rushstack/issues/1050
   // this file is meant to be prepended to the generated dist/src/RouterLink.d.ts
   // @ts-ignore
   ComputedRef,
   // @ts-ignore
   DefineComponent,
   // @ts-ignore
   RendererElement,
   // @ts-ignore
   RendererNode,
   // @ts-ignore
   ComponentOptionsMixin,
   MaybeRef,
-  AnchorHTMLAttributes,
 } from 'vue'
+import type { AnchorHTMLAttributes } from 'vue'
369-379: Typing strategy achieves goal; verify behavior with dynamic boolean custom bindings

The conditional props cleanly allow anchor events/attrs when custom !== true and exclude them when custom === true, and href is properly omitted. One edge case: <RouterLink :custom="someBool" ...> (or TSX custom={someBool} where someBool: boolean) may not be assignable to either branch of the union (true vs false|undefined), causing a TS error. If that’s acceptable, consider documenting it; otherwise, we may need a broader strategy (tradeoff: allowing dynamic booleans typically re-allows anchor attrs).

You can extend the dts test with:
// dynamic boolean: today this likely errors (by design). Decide if we want to support it.
const isCustom: boolean = Math.random() > 0.5
// @ts-expect-error: dynamic boolean not supported by narrowed props
expectError(<RouterLink to="/" custom={isCustom} onFocus={() => {}} />)
// explicit false should allow anchor attrs/handlers
expectTypeOf<JSX.Element>(<RouterLink to="/" custom={false} onFocus={() => {}} target="_blank" />)
// explicit true should forbid anchor attrs/handlers
// @ts-expect-error
expectError(<RouterLink to="/" custom={true} onFocus={() => {}} />)
packages/router/test-dts/components.test-d.tsx (1)
30-40: Great coverage; add a couple of assertions for explicit custom={false} and ARIA/data fallthrough

Tests capture the core behaviors. Consider adding:

acceptance of custom={false} with events/attrs

an ARIA/data attribute to ensure broad fallthrough coverage
// allows when explicitly false
expectTypeOf<JSX.Element>(<RouterLink to="/" custom={false} onFocus={() => {}} target="_blank" />)
// allows ARIA/data attrs
expectTypeOf<JSX.Element>(<RouterLink to="/" aria-label="Home" data-testid="rl" />)

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e7ce331 and 9248c9f.

📒 Files selected for processing (2)

packages/router/src/RouterLink.ts (2 hunks)
packages/router/test-dts/components.test-d.tsx (1 hunks)

🔇 Additional comments (1)

packages/router/src/RouterLink.ts (1)

381-383: Generic constructor change LGTM

This preserves inference in JSX while enabling the conditional props. No runtime impact; aligns with the stated objective.

…s#2484)

pkg-pr-new · 2025-10-08T14:14:08Z

Open in StackBlitz

npm i https://pkg.pr.new/vue-router@2548

commit: b0ac394

codecov · 2025-10-08T14:15:42Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.61%. Comparing base (ccb5e18) to head (b0ac394).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2548   +/-   ##
=======================================
  Coverage   89.61%   89.61%           
=======================================
  Files          46       46           
  Lines        4103     4103           
  Branches     1090     1090           
=======================================
  Hits         3677     3677           
  Misses        421      421           
  Partials        5        5

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

packages/router/test-dts/components.test-d.tsx (1)
30-40: LGTM! Comprehensive type tests for RouterLink anchor behavior.

The new tests correctly verify:

Event handlers (onFocus, onClick) and anchor attributes (target, rel) are allowed when custom is not set

href is always disallowed (derived from to)

Event handlers are disallowed when custom={true}

Optional enhancement: Consider adding a test for custom={false} explicitly to verify that anchor attributes are also allowed in that case (not just when custom is omitted):
expectTypeOf<JSX.Element>(
  <RouterLink to="/" custom={false} onFocus={() => {}} />
)

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9248c9f and b0ac394.

📒 Files selected for processing (2)

packages/router/src/RouterLink.ts (2 hunks)
packages/router/test-dts/components.test-d.tsx (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

packages/router/test-dts/components.test-d.tsx (2)

packages/router/src/RouterLink.ts (1)

RouterLink (361-361)

packages/router/src/index.ts (1)

RouterLink (162-162)

packages/router/src/RouterLink.ts (1)

packages/router/src/index.ts (2)

RouterLinkProps (165-165)

_RouterLinkI (164-164)

⏰ 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: build

🔇 Additional comments (3)

packages/router/src/RouterLink.ts (3)

29-29: LGTM! Correct import for anchor attributes and event handlers.

Importing AnchorHTMLAttributes from Vue provides the complete set of anchor element attributes and event handlers needed for the type enhancement.

363-380: LGTM! Well-designed conditional type for custom prop.

The type definitions correctly implement the generic approach:

_RouterLinkPropsTypedBase: Base props combining Vue component props + RouterLinkProps

RouterLinkPropsTyped<Custom>: Conditional type that:

When Custom extends true: Excludes anchor attributes (custom render mode)

Otherwise: Includes Omit<AnchorHTMLAttributes, 'href'> for anchor attributes/handlers

Always omits href since it's derived from the to prop

The conditional type correctly handles all scenarios via distributivity:

Custom = undefined → allows anchor attributes ✓

Custom = false → allows anchor attributes ✓

Custom = true → disallows anchor attributes ✓

Custom = boolean → union of both branches (correctly permissive) ✓

389-390: LGTM! Generic constructor enables precise type inference.

Making _RouterLinkI generic over Custom with a default of boolean | undefined allows TypeScript to:

Infer Custom = undefined when the custom prop is omitted → anchor attributes allowed

Infer Custom = true when custom is passed → anchor attributes disallowed

This solves the strictTemplates issue while maintaining type safety and backward compatibility.

posva

Thanks!

coderabbitai bot reviewed Sep 9, 2025

View reviewed changes

hodimbokom and others added 3 commits October 8, 2025 16:10

fix(routerlink): allow event handlers under strictTemplates (fix vuej…

ea77ad9

…s#2484)

refactor: simplify

9da13d8

refactor: more

b0ac394

posva force-pushed the fix/routerlink-strict-templates branch from 9248c9f to b0ac394 Compare October 8, 2025 14:12

coderabbitai bot reviewed Oct 8, 2025

View reviewed changes

posva approved these changes Oct 8, 2025

View reviewed changes

posva merged commit 8f35c6d into vuejs:main Oct 8, 2025
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(routerlink): allow event handlers under strictTemplates (fix #2484) #2548

fix(routerlink): allow event handlers under strictTemplates (fix #2484) #2548

hodimbokom commented Sep 9, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

netlify bot commented Sep 9, 2025 •

edited

Loading

Uh oh!

coderabbitai bot commented Sep 9, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

pkg-pr-new bot commented Oct 8, 2025

Uh oh!

codecov bot commented Oct 8, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

posva left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fix(routerlink): allow event handlers under strictTemplates (fix #2484) #2548

fix(routerlink): allow event handlers under strictTemplates (fix #2484) #2548

Conversation

hodimbokom commented Sep 9, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Solution

Tests

Links

Summary by CodeRabbit

Uh oh!

netlify bot commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for vue-router canceled.

Uh oh!

coderabbitai bot commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

pkg-pr-new bot commented Oct 8, 2025

Uh oh!

codecov bot commented Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

posva left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

hodimbokom commented Sep 9, 2025 •

edited by coderabbitai bot

Loading

netlify bot commented Sep 9, 2025 •

edited

Loading

coderabbitai bot commented Sep 9, 2025 •

edited

Loading

codecov bot commented Oct 8, 2025 •

edited

Loading