Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding new types of performance monitoring #10421

Merged
merged 15 commits into from Feb 28, 2020
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
82 changes: 72 additions & 10 deletions packages/next/client/index.js
Expand Up @@ -172,8 +172,14 @@ export default async ({ webpackHMR: passedWebpackHMR } = {}) => {
const { page: app, mod } = await pageLoader.loadPageScript('/_app')
App = app
if (mod && mod.unstable_onPerformanceData) {
onPerfEntry = function({ name, startTime, value, duration }) {
mod.unstable_onPerformanceData({ name, startTime, value, duration })
onPerfEntry = function({ name, startTime, value, duration, entryType }) {
mod.unstable_onPerformanceData({
name,
startTime,
value,
duration,
entryType,
})
}
}

Expand Down Expand Up @@ -323,14 +329,70 @@ function renderReactElement(reactEl, domEl) {

if (onPerfEntry && ST) {
try {
const observer = new PerformanceObserver(list => {
list.getEntries().forEach(onPerfEntry)
})
// Start observing paint entry types.
observer.observe({
type: 'paint',
buffered: true,
})
function getSupportedTypes(types) {
timneutkens marked this conversation as resolved.
Show resolved Hide resolved
if (PerformanceObserver && PerformanceObserver.supportedEntryTypes) {

Choose a reason for hiding this comment

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

if (PerformanceObserver) will error if PerformanceObserver is not supported. I think you want to check if (self.PerformanceObserver) (or something like that).

While I technically it's fine because this is in a try/catch block, I think the code is clearer if you're not relying on that to do feature detection (or if you do, put a comment explaining why).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

return types.filter(type =>
PerformanceObserver.supportedEntryTypes.includes(type)
)
} else {
return []
}
}

function observeSupportedTypes(entryTypes) {
const supportedTypes = getSupportedTypes(entryTypes)

/**
* Layout shift has special handling as this will be sent
* cumulative layout shift.
*/
if (supportedTypes.includes('layout-shift')) {
timneutkens marked this conversation as resolved.
Show resolved Hide resolved
// Stores the current layout shift score for the page.
let cumulativeLayoutShiftScore = 0
const observer = new PerformanceObserver(list => {
for (const entry of list.getEntries()) {
// Only count layout shifts without recent user input.
if (!entry.hadRecentInput) {
cumulativeLayoutShiftScore += entry.value
}
}
})
observer.observe({
type: 'layout-shift',
buffered: true,
})
document.addEventListener('visibilitychange', () => {
if (document.visibilityState === 'hidden') {
// Force any pending records to be dispatched.
observer.takeRecords()
observer.disconnect()
onPerfEntry({
name: 'cumulative-layout-shift',
value: cumulativeLayoutShiftScore,
})
}
})
}

// For all other entry types, we send values to relayer as given by PerformanceObserver
supportedTypes.forEach(type => {
if (type === 'layout-shift') {
return
}
const observer = new PerformanceObserver(list => {
list.getEntries().forEach(onPerfEntry)
})
observer.observe({
type,
buffered: true,
})
})
}
observeSupportedTypes([
'paint',
'largest-contentful-paint',
timneutkens marked this conversation as resolved.
Show resolved Hide resolved
'layout-shift',
])
} catch (e) {
window.addEventListener('load', () => {
performance.getEntriesByType('paint').forEach(onPerfEntry)
Expand Down
5 changes: 4 additions & 1 deletion test/integration/relay-analytics/pages/_app.js
Expand Up @@ -8,5 +8,8 @@ export default class MyApp extends App {}
Method is experimental and will eventually be handled in a Next.js plugin
*/
export function unstable_onPerformanceData(data) {
localStorage.setItem(data.name, data.value || data.startTime)
localStorage.setItem(
data.name || data.entryType,
data.value !== undefined ? data.value : data.startTime
)
}
7 changes: 6 additions & 1 deletion test/integration/relay-analytics/pages/index.js
@@ -1,3 +1,8 @@
export default () => {
return <h1>Hello!</h1>
return (
<div>
<h1>Foo!</h1>
<h2>bar!</h2>
</div>
)
}
17 changes: 16 additions & 1 deletion test/integration/relay-analytics/test/index.test.js
Expand Up @@ -30,13 +30,28 @@ describe('Analytics relayer', () => {
const firstContentfulPaint = parseFloat(
await browser.eval('localStorage.getItem("first-contentful-paint")')
)
expect(h1Text).toMatch(/Hello!/)
const largestContentfulPaint = parseFloat(
await browser.eval('localStorage.getItem("largest-contentful-paint")')
)
let cls = await browser.eval(
'localStorage.getItem("cumulative-layout-shift")'
)
expect(h1Text).toMatch(/Foo!/)
expect(data).not.toBeNaN()
expect(data).toBeGreaterThan(0)
expect(firstPaint).not.toBeNaN()
expect(firstPaint).toBeGreaterThan(0)
expect(firstContentfulPaint).not.toBeNaN()
expect(firstContentfulPaint).toBeGreaterThan(0)
expect(largestContentfulPaint).not.toBeNaN()
expect(largestContentfulPaint).toBeGreaterThan(0)
expect(cls).toBeNull()
// Create an artificial layout shift
await browser.eval('document.querySelector("h1").style.display = "none"')
cls = parseFloat(
await browser.eval('localStorage.getItem("cumulative-layout-shift")')
)
expect(cls).not.toBeNull()
await browser.close()
})
})