Skip to content

fix: validate PerformanceObserver observe options#57030

Open
ya-nsh wants to merge 1 commit into
facebook:mainfrom
ya-nsh:feat/performance-observer-validation
Open

fix: validate PerformanceObserver observe options#57030
ya-nsh wants to merge 1 commit into
facebook:mainfrom
ya-nsh:feat/performance-observer-validation

Conversation

@ya-nsh
Copy link
Copy Markdown
Contributor

@ya-nsh ya-nsh commented Jun 1, 2026

Summary:

Harden PerformanceObserver.observe() option validation so invalid observer configurations fail deterministically before we register a native observer.

This fixes a few user-facing gaps in the current implementation:

  • calling observe() with no options now throws the expected argument error instead of destructuring undefined
  • the entryTypes / type validation messages now match the actual invalid condition
  • empty entryTypes: [] is rejected instead of silently creating an observer that observes nothing
  • entryTypes combined with buffered is rejected before native registration, matching the existing durationThreshold guard

The goal is to prevent malformed performance observer setup from silently succeeding or failing with misleading errors in apps and performance tooling.

Changelog:

[GENERAL] [FIXED] - Validate invalid PerformanceObserver observe option combinations before native registration

Test Plan:

  • git diff --check
  • npx --yes prettier@3.6.2 --no-config --single-quote --bracket-spacing=false --trailing-comma=all --arrow-parens=avoid --parser flow --check packages/react-native/src/private/webapis/performance/PerformanceObserver.js packages/react-native/src/private/webapis/performance/__tests__/PerformanceObserver-itest.js
  • Parsed changed files with flow-parser@0.316.0

Blocked locally:

  • corepack yarn jest packages/react-native/src/private/webapis/performance/__tests__/PerformanceObserver-itest.js --runInBand could not run because dependencies are not installed.
  • corepack yarn install --frozen-lockfile --ignore-scripts failed resolving unpublished main-version packages such as @react-native/metro-babel-transformer@0.87.0-main from the public registry.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 1, 2026
@facebook-github-tools facebook-github-tools Bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Jun 1, 2026
@rubennorte
Copy link
Copy Markdown
Contributor

Thanks for working on this!

empty entryTypes: [] is rejected instead of silently creating an observer that observes nothing

I'm a bit concerned about this one, since this would be against the spec that doesn't define this shouldn't be allowed. Also other vendors don't fail in this case. We shouldn't make existing code break only in RN if we can avoid it.

@rubennorte
Copy link
Copy Markdown
Contributor

Btw, the tests should be run via Fantom:

yarn fantom PerformanceObserver-itest

@ya-nsh ya-nsh force-pushed the feat/performance-observer-validation branch from 3fb1452 to d9f3f54 Compare June 2, 2026 20:35
@ya-nsh
Copy link
Copy Markdown
Contributor Author

ya-nsh commented Jun 2, 2026

Good catch, thanks. I removed the entryTypes: [] rejection so observe({entryTypes: []}) stays allowed, matching the spec and browser behavior. The PR still keeps the other validation fixes for missing options, entryTypes plus type, and entryTypes plus buffered.

Updated commit: d9f3f54

I also tried the Fantom command locally:

yarn fantom PerformanceObserver-itest

I had to expand the sparse checkout for private/react-native-fantom, add Java, and add the Gradle wrapper paths, but the local checkout still cannot complete rn-tester autolinking because the workspace dependencies are not fully installable here (@react-native/metro-babel-transformer@0.87.0-main is not available from the public registry). The GitHub Fantom check is queued on the updated commit now.

@meta-codesync
Copy link
Copy Markdown

meta-codesync Bot commented Jun 3, 2026

@rubennorte has imported this pull request. If you are a Meta employee, you can view this in D107361950.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants