fix(link): preserve unsafe href click handlers

[NathanDrake2406]

Overview

Area	Change
Goal	Keep dangerous next/link hrefs inert without dropping developer click handlers.
Core change	The unsafe Link render branch now preserves the forwarded ref, LinkStatusContext, and user onClick while still omitting href.
Primary files	packages/vinext/src/shims/link.tsx, tests/e2e/app-router/nextjs-compat/javascript-urls.spec.ts
Expected impact	javascript:, data:, and vbscript: Link hrefs remain blocked, but custom interactive handlers continue to work.

Why

A blocked URL should not erase unrelated anchor behavior. Next.js attaches user onClick to the rendered anchor before its internal navigation handling, and its JavaScript URL protections block the dangerous navigation path rather than making the anchor lose user event handlers entirely. Vinext already strips the unsafe href, which is stricter and useful, but the separate unsafe branch accidentally omitted onClick.

Area	Principle / invariant	What this PR changes
Unsafe Link rendering	Dangerous hrefs must not become browser navigations.	Keeps omitting href for unsafe schemes.
Anchor behavior	Developer-provided anchor handlers should survive URL sanitization.	Passes onClick through on the unsafe branch.
Link internals	Unsafe branch should stay structurally close to normal Link rendering where safe.	Preserves the ref and LinkStatusContext wrapper.

What changed

Scenario	Before	After
<Link href="javascript:void(0)" onClick={trackEvent}>	Renders without href, but the custom onClick is dropped.	Renders without href, the custom onClick runs, and the page URL does not change.

Maintainer review path

1. packages/vinext/src/shims/link.tsx: inspect the dangerous href branch and confirm it still does not pass href or call normal navigation logic.
2. tests/fixtures/app-basic/app/nextjs-compat/javascript-urls/link-onclick/page.tsx: fixture proving a custom click handler is observable after hydration.
3. tests/e2e/app-router/nextjs-compat/javascript-urls.spec.ts: regression test asserting no href, click handler execution, and unchanged URL.

Validation

• vp test run tests/link.test.ts
• vp run vinext#build
• PLAYWRIGHT_PROJECT=app-router npx playwright test tests/e2e/app-router/nextjs-compat/javascript-urls.spec.ts -g "preserves custom Link onClick"
• PLAYWRIGHT_PROJECT=app-router npx playwright test tests/e2e/app-router/nextjs-compat/javascript-urls.spec.ts
• vp check
• Commit hook also ran formatting, lint/type checks, and knip.

Risk / compatibility

• Public API: no type or prop surface changes.
• Runtime: limited to the already-dangerous Link branch.
• Security: preserves vinext's existing stricter behavior of omitting the unsafe href; this PR does not make the dangerous URL reachable through Link navigation.
• Compatibility: better matches Next.js user-event semantics while keeping vinext's defense-in-depth sanitization.

Non-goals

• Does not change router push or replace JavaScript URL handling.
• Does not change vinext's intentional blocking of data: and vbscript: Link hrefs.
• Does not attempt to mimic React's exact console/redbox behavior for rendered javascript: anchors.

References

Reference	Why it matters
Next.js Link implementation	Shows user onClick is attached to Link's anchor props before internal navigation handling.
Next.js JavaScript URL detector	Source for the URL safety semantics vinext's detector already references.
Next.js JavaScript URL e2e tests	Confirms JavaScript URL navigation should be blocked across Link and router surfaces.

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/vinext@1264

commit: 1697f07

[james-elicx]

/bigbonk review

[ask-bonk]

Clean fix. The unsafe branch now structurally mirrors the normal branch where it should (ref, LinkStatusContext, intent handlers) and diverges where it must (raw onClick instead of handleClick, no href). The prop ordering is safe — href is Omitted from AnchorHTMLAttributes in LinkProps and destructured before rest, so {...anchorProps} cannot reintroduce it. Prefetching remains correctly disabled via canLinkPrefetch returning false when isDangerous is true.

The E2E test is well-structured: it verifies no href attribute, confirms the click handler fires, and asserts the URL is unchanged. The fixture page is minimal and focused.

---

Minor suggestion (not blocking): The existing unit test at tests/link-navigation.test.ts:794 ("preserves user intent handlers on dangerous inert links") covers onMouseEnter and onTouchStart but not onClick. Since the core fix is about preserving onClick, consider extending that test to also pass an onClick mock and assert it's captured on the rendered anchor props. The E2E test covers the runtime behavior, but a unit-level assertion would make the coverage more explicit and faster to run.

[ask-bonk]

Review submitted. Here's a summary of my findings:

Approved. The fix is correct and well-scoped:

• The unsafe branch now preserves ref (via setRefs), LinkStatusContext, and the user's onClick handler while still omitting the dangerous href — matching the structural shape of the normal branch where appropriate.
• The raw onClick is correctly used instead of handleClick, since there's no navigation to perform on a blocked link.
• No href can leak through {...anchorProps} because it's Omitted from the type and destructured before the rest spread.
• Prefetching remains disabled for dangerous links via canLinkPrefetch.
• The E2E test and fixture are well-designed.

One minor suggestion: extend the existing unit test at tests/link-navigation.test.ts:794 to also cover onClick, since that's the core of this fix.

github run