Skip to content

Commit

Permalink
Address review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
philipwalton committed Sep 28, 2024
1 parent 7abc440 commit c90d104
Show file tree
Hide file tree
Showing 8 changed files with 48 additions and 16 deletions.
9 changes: 7 additions & 2 deletions src/attribution/onCLS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
} from '../types.js';

const getLargestLayoutShiftEntry = (entries: LayoutShift[]) => {
return entries.reduce((a, b) => (a?.value > b.value ? a : b));
return entries.reduce((a, b) => (a.value > b.value ? a : b));
};

const getLargestLayoutShiftSource = (sources: LayoutShiftAttribution[]) => {
Expand Down Expand Up @@ -53,7 +53,12 @@ const attributeCLS = (metric: CLSMetric): CLSMetricWithAttribution => {
}
}

return {...metric, attribution};
// Use `Object.assign()` to ensure the original metric object is returned.
const metricWithAttribution: CLSMetricWithAttribution = Object.assign(
metric,
{attribution},
);
return metricWithAttribution;
};

/**
Expand Down
7 changes: 6 additions & 1 deletion src/attribution/onFCP.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,12 @@ const attributeFCP = (metric: FCPMetric): FCPMetricWithAttribution => {
}
}

return {...metric, attribution};
// Use `Object.assign()` to ensure the original metric object is returned.
const metricWithAttribution: FCPMetricWithAttribution = Object.assign(
metric,
{attribution},
);
return metricWithAttribution;
};

/**
Expand Down
9 changes: 7 additions & 2 deletions src/attribution/onINP.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ const getIntersectingLoAFs = (
start: DOMHighResTimeStamp,
end: DOMHighResTimeStamp,
) => {
const intersectingLoAFs = [];
const intersectingLoAFs: PerformanceLongAnimationFrameTiming[] = [];

for (const loaf of pendingLoAFs) {
// If the LoAF ends before the given start time, ignore it.
Expand Down Expand Up @@ -281,7 +281,12 @@ const attributeINP = (metric: INPMetric): INPMetricWithAttribution => {
loadState: getLoadState(firstEntry.startTime),
};

return {...metric, attribution};
// Use `Object.assign()` to ensure the original metric object is returned.
const metricWithAttribution: INPMetricWithAttribution = Object.assign(
metric,
{attribution},
);
return metricWithAttribution;
};

/**
Expand Down
7 changes: 6 additions & 1 deletion src/attribution/onLCP.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,12 @@ const attributeLCP = (metric: LCPMetric): LCPMetricWithAttribution => {
}
}

return {...metric, attribution};
// Use `Object.assign()` to ensure the original metric object is returned.
const metricWithAttribution: LCPMetricWithAttribution = Object.assign(
metric,
{attribution},
);
return metricWithAttribution;
};

/**
Expand Down
7 changes: 6 additions & 1 deletion src/attribution/onTTFB.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,12 @@ const attributeTTFB = (metric: TTFBMetric): TTFBMetricWithAttribution => {
};
}

return {...metric, attribution};
// Use `Object.assign()` to ensure the original metric object is returned.
const metricWithAttribution: TTFBMetricWithAttribution = Object.assign(
metric,
{attribution},
);
return metricWithAttribution;
};

/**
Expand Down
13 changes: 8 additions & 5 deletions src/lib/getNavigationEntry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,15 @@ export const getNavigationEntry = (): PerformanceNavigationTiming | void => {
// In some cases no value is reported by the browser (for
// privacy/security reasons), and in other cases (bugs) the value is
// negative or is larger than the current page time. Ignore these cases:
// https://github.com/GoogleChrome/web-vitals/issues/137
// https://github.com/GoogleChrome/web-vitals/issues/162
// https://github.com/GoogleChrome/web-vitals/issues/275
// - https://github.com/GoogleChrome/web-vitals/issues/137
// - https://github.com/GoogleChrome/web-vitals/issues/162
// - https://github.com/GoogleChrome/web-vitals/issues/275
if (
navigationEntry?.responseStart > 0 &&
navigationEntry?.responseStart < globalThis.performance?.now()
navigationEntry &&
navigationEntry.responseStart > 0 &&
// Note: if `navigationEnrtry` exists, then we don't need to
// feature-detect `performance.now()`.
navigationEntry.responseStart < performance.now()
) {
return navigationEntry;
}
Expand Down
2 changes: 1 addition & 1 deletion src/lib/observe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export const observe = <K extends keyof PerformanceEntryMap>(
callback(list.getEntries() as PerformanceEntryMap[K]);
});
});
po.observe({type, buffered: true, ...opts} as PerformanceObserverInit);
po.observe({type, buffered: true, ...opts});
return po;
}
} catch {
Expand Down
10 changes: 7 additions & 3 deletions src/onINP.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,12 @@ export const onINP = (
opts: ReportOpts = {},
) => {
// Return if the browser doesn't support all APIs needed to measure INP.
const eventTimingProto = globalThis.PerformanceEventTiming?.prototype;
if (!(eventTimingProto && 'interactionId' in eventTimingProto)) {
if (
!(
globalThis.PerformanceEventTiming &&
'interactionId' in PerformanceEventTiming.prototype
)
) {
return;
}

Expand All @@ -92,7 +96,7 @@ export const onINP = (

const inp = estimateP98LongestInteraction();

if (inp?.latency !== metric.value) {
if (inp && inp.latency !== metric.value) {
metric.value = inp.latency;
metric.entries = inp.entries;
report();
Expand Down

0 comments on commit c90d104

Please sign in to comment.