Fix metrics measurements under new root API (#24110)

Previously, we weren't recording most (all?) of the Next.js measurements like `Next.js-hydration` in Concurrent Mode. This was mainly because the new API doesn't accept a callback.

Instead of special casing this, I've refactored it so that the measurements are just recorded when Root first flushes (via `useLayoutEffect`), which should be more or less the same timing for the old API.

Concurrent Mode is a little trickier for two reasons:

1. Flushes might be (slightly) delayed due to time-slicing and prioritization
2. Selective hydration might skew measurements in cases where full hydration is aborted

I don't have a good answer for those yet, so they'll need to be addressed when the time comes.
This commit is contained in:
Gerald Monaco 2021-04-20 08:37:32 -07:00 committed by GitHub
parent 8d84b08752
commit 1f5f0d313a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 38 additions and 28 deletions

View file

@ -522,28 +522,35 @@ export function renderError(renderErrorProps: RenderErrorProps): Promise<any> {
let reactRoot: any = null
let shouldUseHydrate: boolean = typeof ReactDOM.hydrate === 'function'
function renderReactElement(reactEl: JSX.Element, domEl: HTMLElement): void {
function renderReactElement(
domEl: HTMLElement,
fn: (cb: () => void) => JSX.Element
): void {
// mark start of hydrate/render
if (ST) {
performance.mark('beforeRender')
}
const reactEl = fn(
shouldUseHydrate ? markHydrateComplete : markRenderComplete
)
if (process.env.__NEXT_REACT_MODE !== 'legacy') {
if (!reactRoot) {
const opts = { hydrate: true }
const opts = { hydrate: shouldUseHydrate }
reactRoot =
process.env.__NEXT_REACT_MODE === 'concurrent'
? (ReactDOM as any).unstable_createRoot(domEl, opts)
: (ReactDOM as any).unstable_createBlockingRoot(domEl, opts)
}
reactRoot.render(reactEl)
shouldUseHydrate = false
} else {
// mark start of hydrate/render
if (ST) {
performance.mark('beforeRender')
}
// The check for `.hydrate` is there to support React alternatives like preact
if (shouldUseHydrate) {
ReactDOM.hydrate(reactEl, domEl, markHydrateComplete)
ReactDOM.hydrate(reactEl, domEl)
shouldUseHydrate = false
} else {
ReactDOM.render(reactEl, domEl, markRenderComplete)
ReactDOM.render(reactEl, domEl)
}
}
}
@ -794,8 +801,10 @@ function doRender(input: RenderRouteInfo): Promise<any> {
resolvePromise()
}
onStart()
const elem: JSX.Element = (
<Root callback={onRootCommit}>
<>
<Head callback={onHeadCommit} />
<AppContainer>
<App {...appProps} />
@ -803,33 +812,34 @@ function doRender(input: RenderRouteInfo): Promise<any> {
<RouteAnnouncer />
</Portal>
</AppContainer>
</Root>
</>
)
onStart()
// We catch runtime errors using componentDidCatch which will trigger renderError
renderReactElement(
process.env.__NEXT_STRICT_MODE ? (
<React.StrictMode>{elem}</React.StrictMode>
) : (
elem
),
appElement!
)
renderReactElement(appElement!, (callback) => (
<Root callbacks={[callback, onRootCommit]}>
{process.env.__NEXT_STRICT_MODE ? (
<React.StrictMode>{elem}</React.StrictMode>
) : (
elem
)}
</Root>
))
return renderPromise
}
function Root({
callback,
callbacks,
children,
}: React.PropsWithChildren<{
callback: () => void
callbacks: Array<() => void>
}>): React.ReactElement {
// We use `useLayoutEffect` to guarantee the callback is executed
// as soon as React flushes the update.
React.useLayoutEffect(() => callback(), [callback])
// We use `useLayoutEffect` to guarantee the callbacks are executed
// as soon as React flushes the update
React.useLayoutEffect(() => callbacks.forEach((callback) => callback()), [
callbacks,
])
if (process.env.__NEXT_TEST_MODE) {
// eslint-disable-next-line react-hooks/rules-of-hooks
React.useEffect(() => {

View file

@ -102,10 +102,10 @@ describe('Build Output', () => {
expect(parseFloat(indexFirstLoad)).toBeCloseTo(65.4, 1)
expect(indexFirstLoad.endsWith('kB')).toBe(true)
expect(parseFloat(err404Size)).toBeCloseTo(3.7, 1)
expect(parseFloat(err404Size)).toBeCloseTo(3.69, 1)
expect(err404Size.endsWith('kB')).toBe(true)
expect(parseFloat(err404FirstLoad)).toBeCloseTo(68.5, 0)
expect(parseFloat(err404FirstLoad)).toBeCloseTo(68.8, 0)
expect(err404FirstLoad.endsWith('kB')).toBe(true)
expect(parseFloat(sharedByAll)).toBeCloseTo(65.1, 1)