Skip to content

Conversation

naaa760
Copy link

@naaa760 naaa760 commented Oct 2, 2025

fixes: #84423

What?

  • Fixed an issue where router.push({ scroll: false }) still scrolls to top when using loading.tsx in production mode.

Why?

  • When a loading.tsx file is present, it creates a suspense boundary that shows a loading UI during navigation. When this boundary resolves, it triggers server patches that were resetting the scroll behavior back to true, ignoring the original { scroll: false } option.

How?

  • Modified the shouldScroll logic in handleMutable function to preserve existing scroll state when server patches occur:

// Before:
const shouldScroll = mutable.shouldScroll ?? true

// After:
const shouldScroll = mutable.shouldScroll ?? state.focusAndScrollRef.apply === false ? false : true

@ijjk ijjk added the type: next label Oct 2, 2025
@ijjk
Copy link
Member

ijjk commented Oct 2, 2025

Allow CI Workflow Run

  • approve CI run for commit: 6ef54f8

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

): ReducerState {
// shouldScroll is true by default, can override to false.
const shouldScroll = mutable.shouldScroll ?? true
const shouldScroll = mutable.shouldScroll ?? state.focusAndScrollRef.apply === false ? false : true
Copy link
Contributor

Choose a reason for hiding this comment

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

The expression needs parentheses to ensure correct operator precedence. The nullish coalescing operator (??) has lower precedence than the conditional operator (?:), which could lead to unexpected behavior.

Consider updating to:

const shouldScroll = mutable.shouldScroll ?? (state.focusAndScrollRef.apply === false ? false : true)

This ensures the conditional expression is evaluated as a unit before being used as the fallback value for the nullish coalescing operation.

Suggested change
const shouldScroll = mutable.shouldScroll ?? state.focusAndScrollRef.apply === false ? false : true
const shouldScroll = mutable.shouldScroll ?? (state.focusAndScrollRef.apply === false ? false : true)

Spotted by Diamond

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

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

Successfully merging this pull request may close these issues.

router.push with { scroll: false } still scrolls to top when using loading.tsx in production
2 participants