Skip to content

ref(browser)!: Drop Safari 14 support - remove navigationStart fallback and pagehide listeners - #21293

Merged
msonnb merged 5 commits into
getsentry:developfrom
eddie333016:eddie333016/drop-safari-14-support
Aug 4, 2026
Merged

ref(browser)!: Drop Safari 14 support - remove navigationStart fallback and pagehide listeners#21293
msonnb merged 5 commits into
getsentry:developfrom
eddie333016:eddie333016/drop-safari-14-support

Conversation

@eddie333016

Copy link
Copy Markdown
Contributor

Summary

Closes #18707

Drops Safari 14 compatibility code by removing the performance.timing.navigationStart fallback and pagehide event listeners from the vendored web-vitals code.

Changes

Core timing (packages/core/src/utils/time.ts):

  • Removed performance.timing?.navigationStart fallback in getBrowserTimeOrigin() — Safari 15+ supports performance.timeOrigin natively
  • Updated JSDoc to reflect the simplified fallback chain

Web-vitals vendored code (packages/browser-utils/src/metrics/web-vitals/lib/):

  • onHidden.ts: Removed pagehide event listener, now only listens to visibilitychange
  • getVisibilityWatcher.ts: Removed pagehide event listener, removed isPageHidden() helper, simplified onVisibilityUpdate to only check document.visibilityState
  • whenIdleOrHidden.ts: Removed pagehide add/remove listener pairs

Integration tests:

  • Updated hidePage() helpers to dispatch visibilitychange (with document.visibilityState = 'hidden') instead of pagehide
  • Updated inline pagehide dispatches to use the same pattern
  • Kept sentry.report_event: 'pagehide' data labels unchanged (backend-facing API contract)

Bundle size impact

Saves ~100 bytes by removing unnecessary fallback code paths and event listeners.

@eddie333016
eddie333016 requested a review from a team as a code owner June 2, 2026 13:04
@eddie333016
eddie333016 requested review from logaretm and mydea and removed request for a team June 2, 2026 13:04

@logaretm logaretm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The Safari 14/pagehide removal is fine for v11, but the PR needs to clean up the stale tests.

At any case, this PR won't be merged at the moment until v11 is being prepared for release given its breaking changes nature.

Comment on lines -117 to -123
const navigationStart = performance.timing?.navigationStart;
if (typeof navigationStart === 'number') {
const navigationStartDelta = Math.abs(navigationStart + performanceNow - dateNow);
if (navigationStartDelta < threshold) {
return navigationStart;
}
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some tests still assert the navigationStart. Those tests will fail.

@logaretm
logaretm requested a review from Lms24 June 2, 2026 18:20
@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

👋 @mydea, @Lms24 — Please review this PR when you get a chance!

@eddie333016

Copy link
Copy Markdown
Contributor Author

Thanks for the review @logaretm! You're right — the stale tests would have failed.

I've pushed a follow-up commit that cleans everything up:

  1. Removed the navigationStart fallback test — the code path no longer exists, so this test would have incorrectly expected navigationStart to be returned when it now falls through to Date.now() - performance.now()
  2. Removed timing.navigationStart stubs from the two remaining tests — they're no longer relevant since the production code never reads performance.timing
  3. Updated test descriptions — no longer reference navigationStart
  4. Updated replay test fixturestimeOrigin.mode changed from 'navigationStart' to 'timeOrigin' to match the new code path
  5. Fixed missing trailing newlines in all modified files

Understood on the v11 merge timing — happy for this to wait until the release prep begins.

Comment thread packages/browser-utils/src/metrics/web-vitals/lib/onHidden.ts
@logaretm
logaretm marked this pull request as draft June 8, 2026 16:30
@logaretm

logaretm commented Jun 8, 2026

Copy link
Copy Markdown
Member

Thanks for applying the changes, I will mark it as a draft to prevent merging it by mistake.

@msonnb
msonnb force-pushed the eddie333016/drop-safari-14-support branch from 28e6ed1 to c3f0e2a Compare July 22, 2026 12:20
@msonnb msonnb changed the title ref: Drop Safari 14 support - remove navigationStart fallback and pagehide listeners ref(browser)!: Drop Safari 14 support - remove navigationStart fallback and pagehide listeners Jul 22, 2026
@msonnb
msonnb force-pushed the eddie333016/drop-safari-14-support branch from ff256f6 to 284e5ea Compare July 24, 2026 09:19
@msonnb
msonnb marked this pull request as ready for review July 24, 2026 09:38
Comment thread packages/browser-utils/src/metrics/web-vitals/lib/onHidden.ts Outdated
Comment thread packages/browser-utils/src/metrics/web-vitals/lib/onHidden.ts
eddie333016 and others added 5 commits August 4, 2026 14:08
…rt fallback and pagehide listeners

Safari 15+ has full performance.timeOrigin support and reliable
visibilitychange events. This removes:

- performance.timing?.navigationStart fallback in getBrowserTimeOrigin()
- pagehide event listeners in web-vitals vendored code (onHidden, getVisibilityWatcher, whenIdleOrHidden)
- isPageHidden() helper that checked for pagehide event type
- Updated integration tests to use visibilitychange instead of pagehide

This saves ~100 bytes of unnecessary browser compatibility code.

Closes getsentry#18707
Addresses review feedback from @logaretm:
- Remove test that asserted navigationStart fallback (code path removed)
- Remove navigationStart stubs from remaining tests (no longer relevant)
- Update test descriptions to reflect current behavior
- Update replay fixture timeOrigin.mode from 'navigationStart' to 'timeOrigin'
- Fix missing trailing newlines in all modified files
The Safari 14 support removal dropped the `pagehide` listener from the
vendored web-vitals code, so the React/react-router SPA E2E apps that
flushed the INP span by dispatching a bare `pagehide` event no longer
triggered a flush and timed out. Switch them to the same
`visibilitychange` pattern already used in the browser integration tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@msonnb
msonnb force-pushed the eddie333016/drop-safari-14-support branch from f3d6abc to 043b2ad Compare August 4, 2026 12:16

@msonnb msonnb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

thanks @eddie333016!

@msonnb
msonnb requested a review from logaretm August 4, 2026 12:43
@msonnb
msonnb merged commit c2b9a32 into getsentry:develop Aug 4, 2026
242 checks passed
msonnb added a commit that referenced this pull request Aug 4, 2026
This PR adds the external contributor to the CHANGELOG.md file, so that
they are credited for their contribution. See #21293

Co-authored-by: msonnb <26038247+msonnb@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Drop Support for Safari 14

3 participants