Skip to content

Commit

Permalink
Call Web Vitals reporting at correct time (#17933)
Browse files Browse the repository at this point in the history
This PR adjust the Web Vitals reporting to be called *after* rendering occurs. It used to work like this, but React's render method is no longer synchronous—so we have to do it in an effect.

Existing tests should cover this code path, and IMO it's unfeasible to test that it's invoked _after_ hydration.

Also removed the `&& ST` condition which is not relevant to Web Vitals.
  • Loading branch information
Timer committed Oct 16, 2020
1 parent 4b126cc commit 75f75f3
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 11 deletions.
9 changes: 5 additions & 4 deletions packages/next/client/index.tsx
Expand Up @@ -462,10 +462,6 @@ function renderReactElement(reactEl: JSX.Element, domEl: HTMLElement) {
if (isInitialRender) {
ReactDOM.hydrate(reactEl, domEl, markHydrateComplete)
isInitialRender = false

if (onPerfEntry && ST) {
measureWebVitals(onPerfEntry)
}
} else {
ReactDOM.render(reactEl, domEl, markRenderComplete)
}
Expand Down Expand Up @@ -744,5 +740,10 @@ function Root({
}
}, [])
}
// We should ask to measure the Web Vitals after rendering completes so we
// don't cause any hydration delay:
React.useEffect(() => {
measureWebVitals(onPerfEntry)
}, [])
return children as React.ReactElement
}
31 changes: 25 additions & 6 deletions packages/next/client/performance-relayer.ts
Expand Up @@ -4,13 +4,32 @@ import {
getFID,
getLCP,
getTTFB,
Metric,
ReportHandler,
} from 'web-vitals'

export default (onPerfEntry: ReportHandler) => {
getCLS(onPerfEntry)
getFID(onPerfEntry)
getFCP(onPerfEntry)
getLCP(onPerfEntry)
getTTFB(onPerfEntry)
let isRegistered = false
let userReportHandler: ReportHandler | undefined

function onReport(metric: Metric) {
if (userReportHandler) {
userReportHandler(metric)
}
}

export default (onPerfEntry?: ReportHandler) => {
// Update function if it changes:
userReportHandler = onPerfEntry

// Only register listeners once:
if (isRegistered) {
return
}
isRegistered = true

getCLS(onReport)
getFID(onReport)
getFCP(onReport)
getLCP(onReport)
getTTFB(onReport)
}
2 changes: 1 addition & 1 deletion test/integration/build-output/test/index.test.js
Expand Up @@ -104,7 +104,7 @@ describe('Build Output', () => {
expect(parseFloat(err404FirstLoad) - 64.2).toBeLessThanOrEqual(0)
expect(err404FirstLoad.endsWith('kB')).toBe(true)

expect(parseFloat(sharedByAll) - 60.7).toBeLessThanOrEqual(0)
expect(parseFloat(sharedByAll) - 60.8).toBeLessThanOrEqual(0)
expect(sharedByAll.endsWith('kB')).toBe(true)

if (_appSize.endsWith('kB')) {
Expand Down

0 comments on commit 75f75f3

Please sign in to comment.