diff --git a/packages/next/src/client/components/layout-router.tsx b/packages/next/src/client/components/layout-router.tsx index ae2ee297968f69..309f23f36700bc 100644 --- a/packages/next/src/client/components/layout-router.tsx +++ b/packages/next/src/client/components/layout-router.tsx @@ -16,11 +16,14 @@ import { } from './router-reducer/router-reducer-types' import React, { + Fragment, useContext, use, startTransition, Suspense, useDeferredValue, + useLayoutEffect, + type FragmentInstance, type JSX, } from 'react' import ReactDOM from 'react-dom' @@ -46,6 +49,8 @@ const Activity = process.env.__NEXT_ROUTER_BF_CACHE ? (require('react') as typeof import('react')).unstable_Activity : null! +const enableNewScrollHandler = process.env.__NEXT_APP_NEW_SCROLL_HANDLER + /** * Add refetch marker to router state at the point of the current layout segment. * This ensures the response returned is not further down than the current layout segment. @@ -153,9 +158,23 @@ function shouldSkipElement(element: HTMLElement) { /** * Check if the top corner of the HTMLElement is in the viewport. */ -function topOfElementInViewport(element: HTMLElement, viewportHeight: number) { - const rect = element.getBoundingClientRect() - return rect.top >= 0 && rect.top <= viewportHeight +function topOfElementInViewport( + instance: HTMLElement | FragmentInstance, + viewportHeight: number +) { + const rects = instance.getClientRects() + if (rects.length === 0) { + // Just to be explicit. + return false + } + let elementTop = Number.POSITIVE_INFINITY + for (let i = 0; i < rects.length; i++) { + const rect = rects[i] + if (rect.top < elementTop) { + elementTop = rect.top + } + } + return elementTop >= 0 && elementTop <= viewportHeight } /** @@ -182,7 +201,7 @@ interface ScrollAndFocusHandlerProps { children: React.ReactNode segmentPath: FlightSegmentPath } -class InnerScrollAndFocusHandler extends React.Component { +class InnerScrollAndFocusHandlerOld extends React.Component { handlePotentialScroll = () => { // Handle scroll and focus, it's only applied once. const { focusAndScrollRef, segmentPath } = this.props @@ -227,9 +246,9 @@ class InnerScrollAndFocusHandler extends React.Component(null) + + useLayoutEffect( + () => { + const { focusAndScrollRef, segmentPath } = props + // Handle scroll and focus, it's only applied once in the first useEffect that triggers that changed. + + if (focusAndScrollRef.apply) { + // segmentPaths is an array of segment paths that should be scrolled to + // if the current segment path is not in the array, the scroll is not applied + // unless the array is empty, in which case the scroll is always applied + if ( + focusAndScrollRef.segmentPaths.length !== 0 && + !focusAndScrollRef.segmentPaths.some((scrollRefSegmentPath) => + segmentPath.every((segment, index) => + matchSegment(segment, scrollRefSegmentPath[index]) + ) + ) + ) { + return + } + + let instance: FragmentInstance | HTMLElement | null = null + const hashFragment = focusAndScrollRef.hashFragment + + if (hashFragment) { + instance = getHashFragmentDomNode(hashFragment) + } + + if (!instance) { + instance = childrenRef.current + } + + // If there is no DOM node this layout-router level is skipped. It'll be handled higher-up in the tree. + if (instance === null) { + return + } + + // State is mutated to ensure that the focus and scroll is applied only once. + focusAndScrollRef.apply = false + focusAndScrollRef.hashFragment = null + focusAndScrollRef.segmentPaths = [] + + disableSmoothScrollDuringRouteTransition( + () => { + // In case of hash scroll, we only need to scroll the element into view + if (hashFragment) { + // @ts-expect-error -- Needs `@types/react-dom` update + instance.scrollIntoView() + + return + } + // Store the current viewport height because reading `clientHeight` causes a reflow, + // and it won't change during this function. + const htmlElement = document.documentElement + const viewportHeight = htmlElement.clientHeight + + // If the element's top edge is already in the viewport, exit early. + if (topOfElementInViewport(instance, viewportHeight)) { + return + } + + // Otherwise, try scrolling go the top of the document to be backward compatible with pages + // scrollIntoView() called on `` element scrolls horizontally on chrome and firefox (that shouldn't happen) + // We could use it to scroll horizontally following RTL but that also seems to be broken - it will always scroll left + // scrollLeft = 0 also seems to ignore RTL and manually checking for RTL is too much hassle so we will scroll just vertically + htmlElement.scrollTop = 0 + + // Scroll to domNode if domNode is not in viewport when scrolled to top of document + if (!topOfElementInViewport(instance, viewportHeight)) { + // Scroll into view doesn't scroll horizontally by default when not needed + // @ts-expect-error -- Needs `@types/react-dom` update + instance.scrollIntoView() + } + }, + { + // We will force layout by querying domNode position + dontForceLayout: true, + onlyHashChange: focusAndScrollRef.onlyHashChange, + } + ) + + // Mutate after scrolling so that it can be read by `disableSmoothScrollDuringRouteTransition` + focusAndScrollRef.onlyHashChange = false + + // Set focus on the element + instance.focus() + } + }, + // Used to run on every commit. We may be able to be smarter about this + // but be prepared for lots of manual testing. + undefined + ) + + return {props.children} +} + +const InnerScrollAndFocusHandler = enableNewScrollHandler + ? InnerScrollAndFocusHandlerNew + : InnerScrollAndFocusHandlerOld + function ScrollAndFocusHandler({ segmentPath, children, diff --git a/test/development/acceptance-app/hydration-error.test.ts b/test/development/acceptance-app/hydration-error.test.ts index 01ee69a19bd0e9..7a8ab95d421d48 100644 --- a/test/development/acceptance-app/hydration-error.test.ts +++ b/test/development/acceptance-app/hydration-error.test.ts @@ -5,6 +5,14 @@ import path from 'path' import { outdent } from 'outdent' import { getToastErrorCount, retry } from 'next-test-utils' +const enableNewScrollHandler = Boolean( + process.env.__NEXT_EXPERIMENTAL_APP_NEW_SCROLL_HANDLER +) + +const innerScrollAndFocusHandlerName = enableNewScrollHandler + ? 'InnerScrollAndFocusHandlerNew' + : 'InnerScrollAndFocusHandlerOld' + describe('Error overlay for hydration errors in App router', () => { const { next, isTurbopack } = nextTestSetup({ files: new FileRef(path.join(__dirname, 'fixtures', 'default-template')), @@ -74,7 +82,7 @@ describe('Error overlay for hydration errors in App router', () => { { "componentStack": "... - + <${innerScrollAndFocusHandlerName} segmentPath={[...]} focusAndScrollRef={{apply:false, ...}}> } forbidden={undefined} unauthorized={undefined}> @@ -154,7 +162,7 @@ describe('Error overlay for hydration errors in App router', () => { { "componentStack": "... - + <${innerScrollAndFocusHandlerName} segmentPath={[...]} focusAndScrollRef={{apply:false, ...}}> } forbidden={undefined} unauthorized={undefined}> @@ -311,7 +319,7 @@ describe('Error overlay for hydration errors in App router', () => { { "componentStack": "... - + <${innerScrollAndFocusHandlerName} segmentPath={[...]} focusAndScrollRef={{apply:false, ...}}> } forbidden={undefined} unauthorized={undefined}> @@ -372,7 +380,7 @@ describe('Error overlay for hydration errors in App router', () => { "componentStack": "... - + <${innerScrollAndFocusHandlerName} segmentPath={[...]} focusAndScrollRef={{apply:false, ...}}> } forbidden={undefined} unauthorized={undefined}> @@ -426,7 +434,7 @@ describe('Error overlay for hydration errors in App router', () => { "componentStack": "... - + <${innerScrollAndFocusHandlerName} segmentPath={[...]} focusAndScrollRef={{apply:false, ...}}> } forbidden={undefined} unauthorized={undefined}> @@ -488,7 +496,7 @@ describe('Error overlay for hydration errors in App router', () => { [ { "componentStack": "... - + <${innerScrollAndFocusHandlerName} segmentPath={[...]} focusAndScrollRef={{apply:false, ...}}> } forbidden={undefined} unauthorized={undefined}> @@ -523,7 +531,7 @@ describe('Error overlay for hydration errors in App router', () => { "componentStack": "... - + <${innerScrollAndFocusHandlerName} segmentPath={[...]} focusAndScrollRef={{apply:false, ...}}> } forbidden={undefined} unauthorized={undefined}> @@ -582,7 +590,7 @@ describe('Error overlay for hydration errors in App router', () => { "componentStack": "... - + <${innerScrollAndFocusHandlerName} segmentPath={[...]} focusAndScrollRef={{apply:false, ...}}> } forbidden={undefined} unauthorized={undefined}> @@ -741,7 +749,7 @@ describe('Error overlay for hydration errors in App router', () => { "componentStack": "... - + <${innerScrollAndFocusHandlerName} segmentPath={[...]} focusAndScrollRef={{apply:false, ...}}> } forbidden={undefined} unauthorized={undefined}> @@ -821,7 +829,7 @@ describe('Error overlay for hydration errors in App router', () => { [ { "componentStack": "... - + <${innerScrollAndFocusHandlerName} segmentPath={[...]} focusAndScrollRef={{apply:false, ...}}> } forbidden={undefined} unauthorized={undefined}> @@ -896,7 +904,7 @@ describe('Error overlay for hydration errors in App router', () => { "componentStack": "... - + <${innerScrollAndFocusHandlerName} segmentPath={[...]} focusAndScrollRef={{apply:false, ...}}> } forbidden={undefined} unauthorized={undefined}> @@ -972,7 +980,7 @@ describe('Error overlay for hydration errors in App router', () => { "componentStack": "... - + <${innerScrollAndFocusHandlerName} segmentPath={[...]} focusAndScrollRef={{apply:false, ...}}> } forbidden={undefined} unauthorized={undefined}> diff --git a/test/development/app-dir/hydration-error-count/hydration-error-count.test.ts b/test/development/app-dir/hydration-error-count/hydration-error-count.test.ts index 89d2c0987f5450..e8046a0c01fcec 100644 --- a/test/development/app-dir/hydration-error-count/hydration-error-count.test.ts +++ b/test/development/app-dir/hydration-error-count/hydration-error-count.test.ts @@ -1,5 +1,13 @@ import { nextTestSetup } from 'e2e-utils' +const enableNewScrollHandler = Boolean( + process.env.__NEXT_EXPERIMENTAL_APP_NEW_SCROLL_HANDLER +) + +const innerScrollAndFocusHandlerName = enableNewScrollHandler + ? 'InnerScrollAndFocusHandlerNew' + : 'InnerScrollAndFocusHandlerOld' + describe('hydration-error-count', () => { const { next } = nextTestSetup({ files: __dirname, @@ -14,7 +22,7 @@ describe('hydration-error-count', () => { "componentStack": "... - + <${innerScrollAndFocusHandlerName} segmentPath={[...]} focusAndScrollRef={{apply:false, ...}}> @@ -66,7 +74,7 @@ describe('hydration-error-count', () => { "componentStack": "... - + <${innerScrollAndFocusHandlerName} segmentPath={[...]} focusAndScrollRef={{apply:false, ...}}> @@ -106,7 +114,7 @@ describe('hydration-error-count', () => { "componentStack": "... - + <${innerScrollAndFocusHandlerName} segmentPath={[...]} focusAndScrollRef={{apply:false, ...}}> @@ -138,7 +146,7 @@ describe('hydration-error-count', () => { "componentStack": "... - + <${innerScrollAndFocusHandlerName} segmentPath={[...]} focusAndScrollRef={{apply:false, ...}}> @@ -180,7 +188,7 @@ describe('hydration-error-count', () => { "componentStack": "... - + <${innerScrollAndFocusHandlerName} segmentPath={[...]} focusAndScrollRef={{apply:false, ...}}> diff --git a/test/e2e/app-dir/cache-components-errors/cache-components-errors.test.ts b/test/e2e/app-dir/cache-components-errors/cache-components-errors.test.ts index c0e0d13d7c475c..2c941b28672829 100644 --- a/test/e2e/app-dir/cache-components-errors/cache-components-errors.test.ts +++ b/test/e2e/app-dir/cache-components-errors/cache-components-errors.test.ts @@ -5,6 +5,14 @@ import { getPrerenderOutput, } from './utils' +const enableNewScrollHandler = Boolean( + process.env.__NEXT_EXPERIMENTAL_APP_NEW_SCROLL_HANDLER +) + +const innerScrollAndFocusHandlerName = enableNewScrollHandler + ? 'InnerScrollAndFocusHandlerNew' + : 'InnerScrollAndFocusHandlerOld' + describe('Cache Components Errors', () => { const { next, isTurbopack, isNextStart, skipped } = nextTestSetup({ files: __dirname + '/fixtures/default', @@ -223,7 +231,7 @@ describe('Cache Components Errors', () => { at HTTPAccessFallbackBoundary (bundler:///) at LoadingBoundary (bundler:///) at ErrorBoundary (bundler:///) - at InnerScrollAndFocusHandler (bundler:///) + at ${innerScrollAndFocusHandlerName} (bundler:///) at ScrollAndFocusHandler (bundler:///) at RenderFromTemplateContext (bundler:///) at OuterLayoutRouter (bundler:///) @@ -237,17 +245,17 @@ describe('Cache Components Errors', () => { at HTTPAccessFallbackBoundary (bundler:///) at LoadingBoundary (bundler:///) at ErrorBoundary (bundler:///) - at InnerScrollAndFocusHandler (bundler:///) + at ${innerScrollAndFocusHandlerName} (bundler:///) at ScrollAndFocusHandler (bundler:///) at RenderFromTemplateContext (bundler:///) at OuterLayoutRouter (bundler:///) - 330 | */ - 331 | function InnerLayoutRouter({ - > 332 | tree, + 451 | */ + 452 | function InnerLayoutRouter({ + > 453 | tree, | ^ - 333 | segmentPath, - 334 | cacheNode, - 335 | url, + 454 | segmentPath, + 455 | cacheNode, + 456 | url, To get a more detailed stack trace and pinpoint the issue, start the app in development mode by running \`next dev\`, then open "/dynamic-metadata-error-route" in your browser to investigate the error. Error occurred prerendering page "/dynamic-metadata-error-route". Read more: https://nextjs.org/docs/messages/prerender-error @@ -697,7 +705,7 @@ describe('Cache Components Errors', () => { at HTTPAccessFallbackBoundary (bundler:///) at LoadingBoundary (bundler:///) at ErrorBoundary (bundler:///) - at InnerScrollAndFocusHandler (bundler:///) + at ${innerScrollAndFocusHandlerName} (bundler:///) at ScrollAndFocusHandler (bundler:///) at RenderFromTemplateContext (bundler:///) at OuterLayoutRouter (bundler:///) @@ -711,7 +719,7 @@ describe('Cache Components Errors', () => { at HTTPAccessFallbackBoundary (bundler:///) at LoadingBoundary (bundler:///) at ErrorBoundary (bundler:///) - at InnerScrollAndFocusHandler (bundler:///) + at ${innerScrollAndFocusHandlerName} (bundler:///) at ScrollAndFocusHandler (bundler:///) at RenderFromTemplateContext (bundler:///) at OuterLayoutRouter (bundler:///) @@ -730,7 +738,7 @@ describe('Cache Components Errors', () => { at HTTPAccessFallbackBoundary (bundler:///) at LoadingBoundary (bundler:///) at ErrorBoundary (bundler:///) - at InnerScrollAndFocusHandler (bundler:///) + at ${innerScrollAndFocusHandlerName} (bundler:///) at ScrollAndFocusHandler (bundler:///) at RenderFromTemplateContext (bundler:///) at OuterLayoutRouter (bundler:///) @@ -744,17 +752,17 @@ describe('Cache Components Errors', () => { at HTTPAccessFallbackBoundary (bundler:///) at LoadingBoundary (bundler:///) at ErrorBoundary (bundler:///) - at InnerScrollAndFocusHandler (bundler:///) + at ${innerScrollAndFocusHandlerName} (bundler:///) at ScrollAndFocusHandler (bundler:///) at RenderFromTemplateContext (bundler:///) at OuterLayoutRouter (bundler:///) - 330 | */ - 331 | function InnerLayoutRouter({ - > 332 | tree, + 451 | */ + 452 | function InnerLayoutRouter({ + > 453 | tree, | ^ - 333 | segmentPath, - 334 | cacheNode, - 335 | url, + 454 | segmentPath, + 455 | cacheNode, + 456 | url, To get a more detailed stack trace and pinpoint the issue, start the app in development mode by running \`next dev\`, then open "/dynamic-root" in your browser to investigate the error. Error occurred prerendering page "/dynamic-root". Read more: https://nextjs.org/docs/messages/prerender-error @@ -2105,7 +2113,7 @@ describe('Cache Components Errors', () => { at HTTPAccessFallbackBoundary (bundler:///) at LoadingBoundary (bundler:///) at ErrorBoundary (bundler:///) - at InnerScrollAndFocusHandler (bundler:///) + at ${innerScrollAndFocusHandlerName} (bundler:///) at ScrollAndFocusHandler (bundler:///) at RenderFromTemplateContext () at OuterLayoutRouter (bundler:///) @@ -2118,7 +2126,7 @@ describe('Cache Components Errors', () => { at HTTPAccessFallbackBoundary (bundler:///) at LoadingBoundary (bundler:///) at ErrorBoundary (bundler:///) - at InnerScrollAndFocusHandler (bundler:///) + at ${innerScrollAndFocusHandlerName} (bundler:///) at ScrollAndFocusHandler (bundler:///) at RenderFromTemplateContext () at OuterLayoutRouter (bundler:///) @@ -2129,17 +2137,17 @@ describe('Cache Components Errors', () => { at HTTPAccessFallbackBoundary (bundler:///) at LoadingBoundary (bundler:///) at ErrorBoundary (bundler:///) - at InnerScrollAndFocusHandler (bundler:///) + at ${innerScrollAndFocusHandlerName} (bundler:///) at ScrollAndFocusHandler (bundler:///) at RenderFromTemplateContext () at OuterLayoutRouter (bundler:///) - 330 | */ - 331 | function InnerLayoutRouter({ - > 332 | tree, + 451 | */ + 452 | function InnerLayoutRouter({ + > 453 | tree, | ^ - 333 | segmentPath, - 334 | cacheNode, - 335 | url, + 454 | segmentPath, + 455 | cacheNode, + 456 | url, To get a more detailed stack trace and pinpoint the issue, start the app in development mode by running \`next dev\`, then open "/sync-attribution/unguarded-async-guarded-clientsync" in your browser to investigate the error. Error occurred prerendering page "/sync-attribution/unguarded-async-guarded-clientsync". Read more: https://nextjs.org/docs/messages/prerender-error @@ -3153,7 +3161,7 @@ describe('Cache Components Errors', () => { at HTTPAccessFallbackBoundary (bundler:///) at LoadingBoundary (bundler:///) at ErrorBoundary (bundler:///) - at InnerScrollAndFocusHandler (bundler:///) + at ${innerScrollAndFocusHandlerName} (bundler:///) at ScrollAndFocusHandler (bundler:///) at RenderFromTemplateContext (bundler:///) at OuterLayoutRouter (bundler:///) @@ -3167,17 +3175,17 @@ describe('Cache Components Errors', () => { at HTTPAccessFallbackBoundary (bundler:///) at LoadingBoundary (bundler:///) at ErrorBoundary (bundler:///) - at InnerScrollAndFocusHandler (bundler:///) + at ${innerScrollAndFocusHandlerName} (bundler:///) at ScrollAndFocusHandler (bundler:///) at RenderFromTemplateContext (bundler:///) at OuterLayoutRouter (bundler:///) - 330 | */ - 331 | function InnerLayoutRouter({ - > 332 | tree, + 451 | */ + 452 | function InnerLayoutRouter({ + > 453 | tree, | ^ - 333 | segmentPath, - 334 | cacheNode, - 335 | url, + 454 | segmentPath, + 455 | cacheNode, + 456 | url, To get a more detailed stack trace and pinpoint the issue, start the app in development mode by running \`next dev\`, then open "/use-cache-private-without-suspense" in your browser to investigate the error. Error occurred prerendering page "/use-cache-private-without-suspense". Read more: https://nextjs.org/docs/messages/prerender-error diff --git a/test/e2e/app-dir/router-autoscroll/app/hoisted/page.tsx b/test/e2e/app-dir/router-autoscroll/app/hoisted/page.tsx new file mode 100644 index 00000000000000..3955d626a2c91c --- /dev/null +++ b/test/e2e/app-dir/router-autoscroll/app/hoisted/page.tsx @@ -0,0 +1,13 @@ +export default function Page() { + return ( + <> + + { + // Repeat 500 elements + Array.from({ length: 500 }, (_, i) => ( +
{i}
+ )) + } + + ) +} diff --git a/test/e2e/app-dir/router-autoscroll/app/page.tsx b/test/e2e/app-dir/router-autoscroll/app/page.tsx index a58ac993518abc..25755fdb868548 100644 --- a/test/e2e/app-dir/router-autoscroll/app/page.tsx +++ b/test/e2e/app-dir/router-autoscroll/app/page.tsx @@ -35,6 +35,10 @@ export default function Page() {
To new metadata
+ +
+ To hoisted +
) } diff --git a/test/e2e/app-dir/router-autoscroll/router-autoscroll.test.ts b/test/e2e/app-dir/router-autoscroll/router-autoscroll.test.ts index 22040eafe2152d..2b46ce3a303f2c 100644 --- a/test/e2e/app-dir/router-autoscroll/router-autoscroll.test.ts +++ b/test/e2e/app-dir/router-autoscroll/router-autoscroll.test.ts @@ -4,6 +4,9 @@ import { check, assertNoConsoleErrors, retry } from 'next-test-utils' const isClientSegmentCacheEnabled = process.env.__NEXT_EXPERIMENTAL_PPR === 'true' +const enableNewScrollHandler = Boolean( + process.env.__NEXT_EXPERIMENTAL_APP_NEW_SCROLL_HANDLER +) describe('router autoscrolling on navigation', () => { const { next, isNextDev } = nextTestSetup({ @@ -273,4 +276,27 @@ describe('router autoscrolling on navigation', () => { }) }) }) + + it('should scroll to top even if React hoists children', async () => { + const browser = await webdriver(next.url, '/') + + // scroll to bottom + await browser.eval( + `window.scrollTo(0, ${await browser.eval('document.documentElement.scrollHeight')})` + ) + // Just need to scroll by something + expect(await getTopScroll(browser)).toBeGreaterThan(0) + + await browser.elementByCss('[href="/hoisted"]').click() + expect( + await browser.eval('document.documentElement.scrollHeight') + ).toBeGreaterThan(0) + if (enableNewScrollHandler) { + await waitForScrollToComplete(browser, { x: 0, y: 0 }) + } else { + await expect( + waitForScrollToComplete(browser, { x: 0, y: 0 }) + ).rejects.toThrow() + } + }) }) diff --git a/test/e2e/app-dir/static-generation-status/index.test.ts b/test/e2e/app-dir/static-generation-status/index.test.ts index 12b1081e6cc8b2..6d2c721a3226e0 100644 --- a/test/e2e/app-dir/static-generation-status/index.test.ts +++ b/test/e2e/app-dir/static-generation-status/index.test.ts @@ -1,7 +1,7 @@ import { nextTestSetup } from 'e2e-utils' describe('app-dir static-generation-status', () => { - const { next } = nextTestSetup({ + const { isNextStart, next } = nextTestSetup({ files: __dirname, })