Skip to content

Warn in dev when scroll-behavior: smooth is set on <html> #613

Description

@vivek7405

Problem

The client router scrolls to top on a forward navigation with window.scrollTo({ left: 0, top: 0, behavior: 'instant' }) (packages/core/src/router-client.js, the recordHistory block ~L1728/1731), forcing an instant scroll regardless of the app's CSS scroll-behavior. This is correct and matches other frameworks (Astro uses the identical call; Next.js forces instant by temporarily disabling smooth). But it means a developer who sets html { scroll-behavior: smooth } expecting smooth ROUTE transitions will not get them: webjs overrides it for navigation (smooth still applies to in-page #anchor links via the untouched scrollIntoView path). That silent divergence is worth a dev hint.

Next.js does exactly this: packages/next/src/shared/lib/router/utils/disable-smooth-scroll.ts checks getComputedStyle(html).scrollBehavior === 'smooth' and warnOnce(...) in dev.

Design / approach

Emit a ONE-TIME, DEV-ONLY console.warn from the client router when, at navigation time, the computed scroll-behavior of <html> is smooth. The message should explain that route transitions are forced instant (so this setting only affects in-page anchors), and note that pairing scroll-behavior: smooth with a sticky backdrop-filter header can cause a one-frame flash on iOS during navigation (the lesson from #610). Informational only, no behavior change, never in production, never throws.

Implementation notes (for the implementing agent)

  • Where: packages/core/src/router-client.js. Reuse the existing warnOnce(key, message) helper (L601-607, backed by the warnedKeys Set) so it fires at most once per page. Trigger it from the navigation/scroll path (near the recordHistory scroll-to-top block ~L1718-1733), reading getComputedStyle(document.documentElement).scrollBehavior === 'smooth'.
  • Dev gate: warnOnce currently is NOT env-gated, so gate THIS call on dev explicitly. NODE_ENV is defined on both sides (root AGENTS.md, env section), so process.env.NODE_ENV !== 'production' works in the browser bundle. Confirm against how render-client.js / component.js gate their dev warnings and match that style.
  • Landmines: getComputedStyle forces a style read; only call it on a real foreground nav (not on every popstate/background revalidation) and only once (the warnOnce guard handles repeat). Do NOT fire for in-page hash anchors (where smooth is desired). Never throw if document/getComputedStyle is unavailable (SSR-safety: this path is client-only, but guard defensively).
  • Invariants: no banned prose glyphs (release: bump core/server/cli versions, honest engines fields #11) in the message or comments; packages/ stays plain JS + JSDoc (docs: sweep stale esbuild-loader / Node 20.6 mentions #10).
  • Tests: packages/core/test/routing/router-client.test.js. Assert (a) the warning fires once when getComputedStyle(html).scrollBehavior is smooth on a foreground nav, (b) it does NOT fire when scroll-behavior is auto, (c) it does NOT fire twice, (d) it is suppressed when NODE_ENV==='production'. The suite already stubs globalThis.getComputedStyle-style globals via linkedom; you may need to stub getComputedStyle to return { scrollBehavior: 'smooth' }.
  • Docs: add a note to agent-docs/advanced.md (the scroll-restoration section) and docs/app/docs/client-router/page.ts that nav scroll is forced instant and that scroll-behavior: smooth on <html> only affects in-page anchors (with the iOS backdrop-filter caveat).

Acceptance criteria

  • A dev-only, fire-once console.warn is emitted when <html> computed scroll-behavior is smooth at foreground-nav time
  • No warning when scroll-behavior is not smooth; no warning on repeat navs; no warning in production
  • No warning for in-page hash-anchor navigation
  • Counterfactual: the test asserting "fires once on smooth" fails if the warn call is removed
  • Unit tests cover fire-once, not-smooth, prod-suppressed
  • agent-docs/advanced.md + client-router docs page note the forced-instant nav scroll and the smooth-only-affects-anchors behavior

Activity

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

Metadata

Metadata

Assignees

Labels

enhancementNew feature or request

Type

No type

Projects

  • Status
    Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions