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.

Sign up for GitHub

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

Jump to bottom

Defer execution of observer callbacks #278

Merged
merged 2 commits into from
Nov 14, 2022
Merged

Defer execution of observer callbacks #278

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1949fc4
Defer execution of observer callbacks
philipwalton Nov 13, 2022
3595ee4
Address review feedback
philipwalton Nov 14, 2022
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
7 changes: 6 additions & 1 deletion src/lib/observe.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,12 @@ export const observe = <K extends keyof PerformanceEntryMap>(
try {
if (PerformanceObserver.supportedEntryTypes.includes(type)) {
const po = new PerformanceObserver((list) => {
callback(list.getEntries() as PerformanceEntryMap[K]);
// Delay by a microtask to workaround a bug in Safari where the
// callback is invoked immediately, rather than in a separate task.
// See: https://github.com/GoogleChrome/web-vitals/issues/277
Promise.resolve().then(() => {
callback(list.getEntries() as PerformanceEntryMap[K]);
});
});
po.observe(Object.assign({
type,
Expand Down
24 changes: 5 additions & 19 deletions src/onFCP.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,7 @@ export const onFCP = (onReport: FCPReportCallback, opts?: ReportOpts) => {
const handleEntries = (entries: FCPMetric['entries']) => {
(entries as PerformancePaintTiming[]).forEach((entry) => {
if (entry.name === 'first-contentful-paint') {
if (po) {
po.disconnect();
}
po!.disconnect();
philipwalton marked this conversation as resolved.
Show resolved Hide resolved

// Only report if the page wasn't hidden prior to the first paint.
if (entry.startTime < visibilityWatcher.firstHiddenTime) {
Expand All @@ -60,23 +58,11 @@ export const onFCP = (onReport: FCPReportCallback, opts?: ReportOpts) => {
});
};

// TODO(philipwalton): remove the use of `fcpEntry` once this bug is fixed.
// https://bugs.webkit.org/show_bug.cgi?id=225305
// The check for `getEntriesByName` is needed to support Opera:
// https://github.com/GoogleChrome/web-vitals/issues/159
// The check for `window.performance` is needed to support Opera mini:
// https://github.com/GoogleChrome/web-vitals/issues/185
philipwalton marked this conversation as resolved.
Show resolved Hide resolved
const fcpEntry = window.performance && window.performance.getEntriesByName &&
window.performance.getEntriesByName('first-contentful-paint')[0];

const po = fcpEntry ? null : observe('paint', handleEntries);

if (fcpEntry || po) {
report = bindReporter(onReport, metric, thresholds, opts.reportAllChanges);
const po = observe('paint', handleEntries);

if (fcpEntry) {
handleEntries([fcpEntry]);
}
if (po) {
report = bindReporter(
onReport, metric, thresholds, opts!.reportAllChanges);

// Only report after a bfcache restore if the `PerformanceObserver`
// successfully registered or the `paint` entry exists.
Expand Down