ref(browser): Restructure browser-utils by domain - #23071
Open
logaretm wants to merge 1 commit into
Open
Conversation
logaretm
force-pushed
the
awad/webvitals-restructure
branch
from
August 5, 2026 16:47
af2fd8a to
cc6aaf8
Compare
Contributor
size-limit report 📦
|
logaretm
force-pushed
the
awad/webvitals-restructure
branch
from
August 5, 2026 17:35
cc6aaf8 to
071799f
Compare
logaretm
force-pushed
the
awad/webvitals-restructure
branch
2 times, most recently
from
August 5, 2026 19:00
e1990b4 to
794433e
Compare
logaretm
force-pushed
the
awad/webvitals-restructure
branch
from
August 5, 2026 19:16
794433e to
7a246f5
Compare
logaretm
force-pushed
the
awad/webvitals-restructure
branch
from
August 5, 2026 19:44
7a246f5 to
12d0fc5
Compare
logaretm
marked this pull request as ready for review
August 5, 2026 20:05
Reorganize browser-utils/src away from the catch-all metrics/ folder into clear domains: - instrumentation/ for event-source hooks (dom, history, location, xhr) plus the PerformanceObserver layer, renamed from metrics/instrument.ts to instrumentation/performanceObserver.ts to stop colliding with the folder - web-vitals/ for web-vital tracking, spans, inp, lcp, reportEvents, and the browser helpers merged into a single utils.ts - performance/ for generic performance-entry enrichment: entries, element/ user/resource timing, and shared utils browserMetrics.ts is split into web-vitals/tracking.ts and performance/ entries.ts, and utils.ts into web-vitals/reportEvents.ts and performance/ utils.ts. Public API (index.ts re-exports) is unchanged; only internal paths and the test tree move.
logaretm
force-pushed
the
awad/webvitals-restructure
branch
from
August 6, 2026 14:06
12d0fc5 to
7c1f6aa
Compare
Comment on lines
+27
to
+32
| function _runCollectorCallbackOnce(event: WebVitalReportEvent) { | ||
| if (!collected && pageloadSpan) { | ||
| collectorCallback(event, pageloadSpan.spanContext().spanId, pageloadSpan); | ||
| } | ||
| collected = true; | ||
| } |
Contributor
There was a problem hiding this comment.
Bug: A race condition during initialization can cause web vitals to be dropped if the page is hidden, because collected is set to true before pageloadSpan is available.
Severity: MEDIUM
Suggested Fix
In the _runCollectorCallbackOnce function, move the collected = true assignment inside the if (!collected && pageloadSpan) block. This will ensure the collected flag is only set when data is actually reported, allowing the callback to run again if it fires prematurely.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/browser-utils/src/web-vitals/reportEvents.ts#L27-L32
Potential issue: A race condition exists between the setup of web vitals listeners and
the creation of the `pageloadSpan`. The `onHidden` callback, which collects web vitals,
is registered before `pageloadSpan` is available. If a user hides the page during this
initialization window, the callback fires. Inside the callback, `pageloadSpan` is
`undefined`, so the collection logic is skipped. However, a `collected` flag is
unconditionally set to `true`. This prevents the web vitals from being collected later
when `pageloadSpan` does become available, leading to silent data loss for metrics like
LCP and CLS.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reorganizes
browser-utils/srcaway from the catch-allmetrics/folder into clear domains:instrumentation/(dom/history/location/xhr + the PerformanceObserver layer),web-vitals/(tracking, spans, inp, lcp, reportEvents, and the helpers merged into one utils), andperformance/(entries, element/user/resource timing, shared utils).The reasoning is we will actually have some metric emitting logic in there and we don't want it to be confused with pre-existing logic like in #22397
Following up on pulling in
webvitalsas a dependency, I took this chance to clean up our structure and it does have some minor bundle-size improvements. The bundle increases are due to #23070 which explains where the increase is coming from and why we are willing to absorb it.