Skip to content

fix(link): preserve native download clicks#1262

Open
NathanDrake2406 wants to merge 1 commit into
cloudflare:mainfrom
NathanDrake2406:nathan/fix-link-download
Open

fix(link): preserve native download clicks#1262
NathanDrake2406 wants to merge 1 commit into
cloudflare:mainfrom
NathanDrake2406:nathan/fix-link-download

Conversation

@NathanDrake2406
Copy link
Copy Markdown
Contributor

Overview

Field Details
Goal Match Next.js <Link download> click semantics.
Core change Return from the Link click handler before preventDefault() when the rendered anchor has the download attribute.
Primary files packages/vinext/src/shims/link.tsx, tests/link-navigation.test.ts
Impact Native file downloads keep browser behavior instead of being intercepted as vinext SPA navigation.

Why

next/link extends an anchor, but an anchor with download is not a client-side route transition. Next.js treats that click as browser-owned behavior: user onClick can run, but onNavigate and router navigation must not.

What changed

Scenario Before After
<Link href="/file.pdf" download> Vinext called preventDefault() and entered SPA navigation. Vinext leaves the browser default download behavior intact.
onClick on a download Link Ran before vinext navigation. Still runs.
onNavigate on a download Link Could run because the click was treated as client navigation. Does not run.

Validation

  • vp test run tests/link-navigation.test.ts -t "download links" failed before the production guard, then passed after it.
  • vp test run tests/link-navigation.test.ts
  • vp check
  • Commit hook reran formatting, lint, type checks, and knip.

References

Reference Why it matters
Next.js App Router Link download guard App Router returns before SPA navigation when the anchor has download.
Next.js Pages Router Link download guard Pages Router uses the same native-browser download rule.
Next.js onNavigate download test Verifies download links update onClick state but not onNavigate state.
Next.js Link docs for onNavigate Documents that download links work with onClick but not onNavigate.

Risk

Low. The new guard only applies when the actual clicked anchor has the HTML download attribute, and it executes after user onClick so preventDefault() from user code still wins. It runs before router-specific App or Pages logic, so both routers share the same compatibility fix.

Link currently treats anchors with the HTML download attribute like normal same-origin SPA navigations. That prevents the browser from performing its native download behavior and can also run onNavigate for a click that should never become client-side navigation.

The click handler now bails out after user onClick and before preventDefault whenever the anchor has a download attribute, matching Next.js Link semantics. A focused regression test covers the default action, onNavigate, transition, and RSC navigate boundaries.
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 16, 2026

Open in StackBlitz

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

commit: 27fc775

@NathanDrake2406 NathanDrake2406 marked this pull request as ready for review May 16, 2026 12:25
@james-elicx
Copy link
Copy Markdown
Collaborator

/bigbonk review

Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean, well-scoped fix. The download guard matches Next.js semantics in both App Router (linkClicked in packages/next/src/client/app-dir/link.tsx) and Pages Router (linkClicked in packages/next/src/client/link.tsx), where the hasAttribute('download') check runs before e.preventDefault() and before onNavigate.

The placement after onClick / defaultPrevented and before the modifier-key check is correct — user onClick still fires, but SPA navigation and onNavigate are skipped.

Test is thorough: covers onClick firing, defaultPrevented staying false, onNavigate not called, startTransition not called, and navigate not called. The Next.js test reference comment is appreciated.

No issues found.

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented May 16, 2026

Approved. The fix is correct and well-tested.

Summary of the review:

  • The download attribute guard in link.tsx:446 is placed at the right position in the click handler — after user onClick fires (so user code can still call preventDefault), but before e.preventDefault() and onNavigate. This exactly matches how Next.js handles it in both the App Router and Pages Router linkClicked functions.
  • The test covers all the important assertions: onClick fires, defaultPrevented remains false, and onNavigate/startTransition/navigate are all skipped.
  • The CapturedClickEvent type update to include hasAttribute on currentTarget is a necessary side-effect of the production code now using that method.

github run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants