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

Update test and stack frame code to support newer V8 stack formats #22477

Merged
merged 2 commits into from
Oct 11, 2021
Merged

Update test and stack frame code to support newer V8 stack formats #22477

merged 2 commits into from
Oct 11, 2021

Conversation

bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Sep 30, 2021 •
edited
Loading

Different versions of V8 use different Error message formats when script tries to read properties from null or undefined. Older versions use e.g. "Cannot read property 'foo' of undefined". Newer versions use e.g. "Cannot read properties of undefined (reading 'foo')".

This PR overrides the built-in toThrow matches to detect which message format is used and match appropriately. (This enables our tests to pass on Node 12-16 without forking.)

This PR also updates describeNativeComponentFrame() to handle another different between engine versions. Specifically, we detect when a function component with a displayName is represented in the component stack as "<anonymous>" and sub in the display name accordingly.

These could be split into separate PRs, but technically both are required for all tests to pass on Node 16, so...

To test this PR, I ran all tests using node 14.17.0 and 16.9.1.

Resolves #22441

@facebook-github-bot facebook-github-bot added the React Core Team Opened by a member of the React Core Team label Sep 30, 2021
@bvaughn bvaughn mentioned this pull request Sep 30, 2021
Closed
@bvaughn bvaughn changed the title Updated tests to support newer V8 stack formats Update test and stack frame code to support newer V8 stack formats Sep 30, 2021
@bvaughn bvaughn requested a review from salazarm September 30, 2021 18:16
@sizebot
Copy link

sizebot commented Sep 30, 2021

Comparing: 75b9869...dabc7fd

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js +0.07% 130.22 kB 130.32 kB +0.10% 41.45 kB 41.49 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js +0.07% 133.05 kB 133.15 kB +0.10% 42.42 kB 42.46 kB
facebook-www/ReactDOM-prod.classic.js +0.05% 412.61 kB 412.82 kB +0.08% 76.37 kB 76.43 kB
facebook-www/ReactDOM-prod.modern.js +0.05% 401.23 kB 401.45 kB +0.08% 74.67 kB 74.72 kB
facebook-www/ReactDOMForked-prod.classic.js +0.05% 412.61 kB 412.82 kB +0.08% 76.38 kB 76.44 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-experimental/react/cjs/react-jsx-dev-runtime.development.js +0.82% 41.78 kB 42.12 kB +0.89% 12.20 kB 12.31 kB
oss-stable-semver/react/cjs/react-jsx-dev-runtime.development.js +0.82% 41.82 kB 42.16 kB +0.88% 12.22 kB 12.33 kB
oss-stable/react/cjs/react-jsx-dev-runtime.development.js +0.82% 41.82 kB 42.16 kB +0.88% 12.22 kB 12.33 kB
oss-experimental/react/cjs/react-jsx-runtime.development.js +0.81% 42.38 kB 42.72 kB +0.88% 12.38 kB 12.49 kB
oss-stable-semver/react/cjs/react-jsx-runtime.development.js +0.81% 42.42 kB 42.76 kB +0.88% 12.39 kB 12.50 kB
oss-stable/react/cjs/react-jsx-runtime.development.js +0.81% 42.42 kB 42.76 kB +0.88% 12.39 kB 12.50 kB
facebook-www/JSXDEVRuntime-dev.modern.js +0.75% 45.49 kB 45.84 kB +0.85% 12.86 kB 12.97 kB
facebook-www/JSXDEVRuntime-dev.classic.js +0.75% 45.49 kB 45.84 kB +0.85% 12.86 kB 12.97 kB
oss-experimental/react/cjs/react-unstable-shared-subset.development.js +0.46% 75.05 kB 75.39 kB +0.49% 20.57 kB 20.67 kB
oss-stable-semver/react/cjs/react.development.js +0.39% 88.43 kB 88.77 kB +0.46% 23.86 kB 23.97 kB
oss-stable/react/cjs/react.development.js +0.39% 88.43 kB 88.77 kB +0.46% 23.86 kB 23.97 kB
oss-experimental/react/cjs/react.development.js +0.38% 89.36 kB 89.70 kB +0.40% 24.01 kB 24.10 kB
oss-stable-semver/react/umd/react.development.js +0.31% 112.15 kB 112.50 kB +0.36% 28.85 kB 28.95 kB
oss-stable/react/umd/react.development.js +0.31% 112.15 kB 112.50 kB +0.36% 28.85 kB 28.95 kB
oss-experimental/react/umd/react.development.js +0.31% 113.12 kB 113.47 kB +0.35% 28.98 kB 29.08 kB
facebook-www/React-dev.modern.js +0.30% 113.52 kB 113.86 kB +0.34% 28.00 kB 28.10 kB
facebook-www/React-dev.classic.js +0.30% 114.53 kB 114.88 kB +0.33% 28.19 kB 28.29 kB
oss-stable-semver/react-server/cjs/react-server.development.js +0.30% 115.91 kB 116.26 kB +0.39% 28.46 kB 28.57 kB
oss-stable/react-server/cjs/react-server.development.js +0.30% 115.91 kB 116.26 kB +0.39% 28.46 kB 28.57 kB
oss-experimental/react-server/cjs/react-server.development.js +0.29% 116.47 kB 116.82 kB +0.38% 28.61 kB 28.72 kB
facebook-www/ReactDOMServer-dev.classic.js +0.22% 158.81 kB 159.15 kB +0.25% 40.25 kB 40.35 kB

Generated by 🚫 dangerJS against dabc7fd

@bvaughn bvaughn requested a review from gaearon September 30, 2021 18:21
@bvaughn bvaughn mentioned this pull request Sep 30, 2021
Open
akgupta0777
akgupta0777 reviewed Oct 1, 2021
View reviewed changes
Copy link
Contributor

@akgupta0777 akgupta0777 left a comment

Choose a reason for hiding this comment

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

Seems good.

scripts/jest/matchers/toThrow.js Show resolved Hide resolved
@bvaughn bvaughn requested a review from jstejada October 1, 2021 13:29
jstejada
jstejada approved these changes Oct 11, 2021
View reviewed changes
Copy link
Contributor

@jstejada jstejada left a comment

Choose a reason for hiding this comment

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

looks good, thanks!

@bvaughn bvaughn merged commit c16b005 into facebook:main Oct 11, 2021
@bvaughn bvaughn deleted the support-v8-error-format-changes branch October 11, 2021 22:40
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Nov 2, 2021
@facebook-github-bot
React Native sync for revisions afcb9cd...3fcd81d
e161598 
Summary:
This sync includes the following changes:
- **[5cccacd13](facebook/react@5cccacd13 )**: Upgrade useId to alpha channel ([#22674](facebook/react#22674)) //<Andrew Clark>//
- **[75f3ddebf](facebook/react@75f3ddebf )**: Remove experimental useOpaqueIdentifier API ([#22672](facebook/react#22672)) //<Andrew Clark>//
- **[8c4a05b8f](facebook/react@8c4a05b8f )**: Remove flow pragma comment from module registration start/stop templates ([#22670](facebook/react#22670)) //<Brian Vaughn>//
- **[ebf9ae857](facebook/react@ebf9ae857 )**: useId ([#22644](facebook/react#22644)) //<Andrew Clark>//
- **[a0d991fe6](facebook/react@a0d991fe6 )**: Re-land #22292 (remove uMS from open source build) ([#22664](facebook/react#22664)) //<Andrew Clark>//
- **[6bce0355c](facebook/react@6bce0355c )**: Upgrade useSyncExternalStore to alpha channel ([#22662](facebook/react#22662)) //<Andrew Clark>//
- **[7034408ff](facebook/react@7034408ff )**: Follow-up improvements to error code extraction infra ([#22516](facebook/react#22516)) //<Andrew Clark>//
- **[90e5d3638](facebook/react@90e5d3638 )**: chore: fix comment typo ([#22615](facebook/react#22615)) //<btea>//
- **[3c4c1c470](facebook/react@3c4c1c470 )**: Remove warning for dangling passive effects ([#22609](facebook/react#22609)) //<Andrew Clark>//
- **[d5b6b4b86](facebook/react@d5b6b4b86 )**: Expand act warning to cover all APIs that might schedule React work ([#22607](facebook/react#22607)) //<Andrew Clark>//
- **[fa9bea0c4](facebook/react@fa9bea0c4 )**: Initial implementation of cache cleanup ([#22510](facebook/react#22510)) //<Joseph Savona>//
- **[0e8a5aff3](facebook/react@0e8a5aff3 )**: Scheduling Profiler: Add marks for component effects (mount and unmount) ([#22578](facebook/react#22578)) //<Brian Vaughn>//
- **[4ba20579d](facebook/react@4ba20579d )**: Scheduling Profiler: De-emphasize React internal frames ([#22588](facebook/react#22588)) //<Brian Vaughn>//
- **[cdb8a1d19](facebook/react@cdb8a1d19 )**: [Fizz] Add option to inject bootstrapping script tags after the shell is injected ([#22594](facebook/react#22594)) //<Sebastian Markbåge>//
- **[34e4c9756](facebook/react@34e4c9756 )**: Clear extra nodes if there's a hydration mismatch within a suspense boundary  ([#22592](facebook/react#22592)) //<Sebastian Markbåge>//
- **[02f411578](facebook/react@02f411578 )**: Upgrade useInsertionEffect to stable ([#22589](facebook/react#22589)) //<Andrew Clark>//
- **[2af4a7933](facebook/react@2af4a7933 )**: Hydrate using SuspenseComponent as the parent ([#22582](facebook/react#22582)) //<Sebastian Markbåge>//
- **[b1acff0cc](facebook/react@b1acff0cc )**: Enable cache in test renderer ([#22580](facebook/react#22580)) //<Joseph Savona>//
- **[996da67b2](facebook/react@996da67b2 )**: Replace global `jest` heuristic with `IS_REACT_ACT_ENVIRONMENT` ([#22562](facebook/react#22562)) //<Andrew Clark>//
- **[163e81c1f](facebook/react@163e81c1f )**: Support disabling spurious act warnings with a global environment flag ([#22561](facebook/react#22561)) //<Andrew Clark>//
- **[23b7dfeff](facebook/react@23b7dfeff )**: Enable scheduling profiler for RN FB profiling builds ([#22566](facebook/react#22566)) //<Brian Vaughn>//
- **[61455a25b](facebook/react@61455a25b )**: Enable experimental Cache API in www TestRenderer ([#22554](facebook/react#22554)) //<Joseph Savona>//
- **[7142d110b](facebook/react@7142d110b )**: Bugfix: Nested useOpaqueIdentifier references ([#22553](facebook/react#22553)) //<Andrew Clark>//
- **[1e247ff89](facebook/react@1e247ff89 )**: Enabled scheduling profiler marks for React Native FB target ([#22544](facebook/react#22544)) //<Brian Vaughn>//
- **[c16b005f2](facebook/react@c16b005f2 )**: Update test and stack frame code to support newer V8 stack formats ([#22477](facebook/react#22477)) //<Brian Vaughn>//
- **[55d75005b](facebook/react@55d75005b )**: duplicate value in variable ([#22390](facebook/react#22390)) //<BIKI DAS>//

Changelog:
[General][Changed] - React Native sync for revisions afcb9cd...3fcd81d

jest_e2e[run_all_tests]

Reviewed By: yungsters

Differential Revision: D32065987

fbshipit-source-id: ef2d402835c981aab68ca40a894c66c1630864e9
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Nov 15, 2021
@facebook-github-bot
React Native sync for revisions afcb9cd...c0c71a8
55663ec 
Summary:
This sync includes the following changes:
- **[c0c71a868](facebook/react@c0c71a868 )**: Re-enable useMutableSource in internal RN ([#22750](facebook/react#22750)) //<Ricky>//
- **[cb11155c8](facebook/react@cb11155c8 )**: Add runtime type checks around module boundary code ([#22748](facebook/react#22748)) //<Brian Vaughn>//
- **[a04f13d29](facebook/react@a04f13d29 )**: react-refresh@0.11.0 //<Dan Abramov>//
- **[ff9897d23](facebook/react@ff9897d23 )**: [React Refresh] support typescript namespace syntax ([#22621](facebook/react#22621)) //<irinakk>//
- **[0ddd69d12](facebook/react@0ddd69d12 )**: Throw on hydration mismatch and force client rendering if boundary hasn't suspended within concurrent root ([#22629](facebook/react#22629)) //<salazarm>//
- **[c3f34e4be](facebook/react@c3f34e4be )**: eslint-plugin-react-hooks@4.3.0 //<Dan Abramov>//
- **[827021c4e](facebook/react@827021c4e )**: Changelog for eslint-plugin-react-hooks@4.3.0 //<Dan Abramov>//
- **[8ca3f567b](facebook/react@8ca3f567b )**: Fix module-boundary wrappers ([#22688](facebook/react#22688)) //<Brian Vaughn>//
- **[1bf6deb86](facebook/react@1bf6deb86 )**: Renamed packages/react-devtools-scheduling-profiler to packages/react-devtools-timeline ([#22691](facebook/react#22691)) //<Brian Vaughn>//
- **[51c558aeb](facebook/react@51c558aeb )**: Rename (some) "scheduling profiler" references to "timeline" ([#22690](facebook/react#22690)) //<Brian Vaughn>//
- **[00ced1e2b](facebook/react@00ced1e2b )**: Fix useId in strict mode ([#22681](facebook/react#22681)) //<Andrew Clark>//
- **[5cccacd13](facebook/react@5cccacd13 )**: Upgrade useId to alpha channel ([#22674](facebook/react#22674)) //<Andrew Clark>//
- **[75f3ddebf](facebook/react@75f3ddebf )**: Remove experimental useOpaqueIdentifier API ([#22672](facebook/react#22672)) //<Andrew Clark>//
- **[8c4a05b8f](facebook/react@8c4a05b8f )**: Remove flow pragma comment from module registration start/stop templates ([#22670](facebook/react#22670)) //<Brian Vaughn>//
- **[ebf9ae857](facebook/react@ebf9ae857 )**: useId ([#22644](facebook/react#22644)) //<Andrew Clark>//
- **[a0d991fe6](facebook/react@a0d991fe6 )**: Re-land #22292 (remove uMS from open source build) ([#22664](facebook/react#22664)) //<Andrew Clark>//
- **[6bce0355c](facebook/react@6bce0355c )**: Upgrade useSyncExternalStore to alpha channel ([#22662](facebook/react#22662)) //<Andrew Clark>//
- **[7034408ff](facebook/react@7034408ff )**: Follow-up improvements to error code extraction infra ([#22516](facebook/react#22516)) //<Andrew Clark>//
- **[90e5d3638](facebook/react@90e5d3638 )**: chore: fix comment typo ([#22615](facebook/react#22615)) //<btea>//
- **[3c4c1c470](facebook/react@3c4c1c470 )**: Remove warning for dangling passive effects ([#22609](facebook/react#22609)) //<Andrew Clark>//
- **[d5b6b4b86](facebook/react@d5b6b4b86 )**: Expand act warning to cover all APIs that might schedule React work ([#22607](facebook/react#22607)) //<Andrew Clark>//
- **[fa9bea0c4](facebook/react@fa9bea0c4 )**: Initial implementation of cache cleanup ([#22510](facebook/react#22510)) //<Joseph Savona>//
- **[0e8a5aff3](facebook/react@0e8a5aff3 )**: Scheduling Profiler: Add marks for component effects (mount and unmount) ([#22578](facebook/react#22578)) //<Brian Vaughn>//
- **[4ba20579d](facebook/react@4ba20579d )**: Scheduling Profiler: De-emphasize React internal frames ([#22588](facebook/react#22588)) //<Brian Vaughn>//
- **[cdb8a1d19](facebook/react@cdb8a1d19 )**: [Fizz] Add option to inject bootstrapping script tags after the shell is injected ([#22594](facebook/react#22594)) //<Sebastian Markbåge>//
- **[34e4c9756](facebook/react@34e4c9756 )**: Clear extra nodes if there's a hydration mismatch within a suspense boundary  ([#22592](facebook/react#22592)) //<Sebastian Markbåge>//
- **[02f411578](facebook/react@02f411578 )**: Upgrade useInsertionEffect to stable ([#22589](facebook/react#22589)) //<Andrew Clark>//
- **[2af4a7933](facebook/react@2af4a7933 )**: Hydrate using SuspenseComponent as the parent ([#22582](facebook/react#22582)) //<Sebastian Markbåge>//
- **[b1acff0cc](facebook/react@b1acff0cc )**: Enable cache in test renderer ([#22580](facebook/react#22580)) //<Joseph Savona>//
- **[996da67b2](facebook/react@996da67b2 )**: Replace global `jest` heuristic with `IS_REACT_ACT_ENVIRONMENT` ([#22562](facebook/react#22562)) //<Andrew Clark>//
- **[163e81c1f](facebook/react@163e81c1f )**: Support disabling spurious act warnings with a global environment flag ([#22561](facebook/react#22561)) //<Andrew Clark>//
- **[23b7dfeff](facebook/react@23b7dfeff )**: Enable scheduling profiler for RN FB profiling builds ([#22566](facebook/react#22566)) //<Brian Vaughn>//
- **[61455a25b](facebook/react@61455a25b )**: Enable experimental Cache API in www TestRenderer ([#22554](facebook/react#22554)) //<Joseph Savona>//
- **[7142d110b](facebook/react@7142d110b )**: Bugfix: Nested useOpaqueIdentifier references ([#22553](facebook/react#22553)) //<Andrew Clark>//
- **[1e247ff89](facebook/react@1e247ff89 )**: Enabled scheduling profiler marks for React Native FB target ([#22544](facebook/react#22544)) //<Brian Vaughn>//
- **[c16b005f2](facebook/react@c16b005f2 )**: Update test and stack frame code to support newer V8 stack formats ([#22477](facebook/react#22477)) //<Brian Vaughn>//
- **[55d75005b](facebook/react@55d75005b )**: duplicate value in variable ([#22390](facebook/react#22390)) //<BIKI DAS>//

Changelog:
[General][Changed] - React Native sync for revisions afcb9cd...c0c71a8

jest_e2e[run_all_tests]

Reviewed By: yungsters

Differential Revision: D32395873

fbshipit-source-id: 3afd158f167b1eedcc244e29aba1a2c502d3c9d9
zhengjitf pushed a commit to zhengjitf/react that referenced this pull request Apr 15, 2022
@zhengjitf
Update test and stack frame code to support newer V8 stack formats (f…
b0ea15d 
…acebook#22477)
@0xdevalias 0xdevalias mentioned this pull request Jul 13, 2022
Closed
@eps1lon eps1lon mentioned this pull request Sep 27, 2022
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akgupta0777 akgupta0777 akgupta0777 left review comments

@jstejada jstejada jstejada approved these changes

@sebmarkbage sebmarkbage Awaiting requested review from sebmarkbage

@rickhanlonii rickhanlonii Awaiting requested review from rickhanlonii

@acdlite acdlite Awaiting requested review from acdlite

@josephsavona josephsavona Awaiting requested review from josephsavona

@salazarm salazarm Awaiting requested review from salazarm

@gaearon gaearon Awaiting requested review from gaearon

Assignees
No one assigned
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Several tests fail on main with Node v16
5 participants
@bvaughn @sizebot @jstejada @akgupta0777 @facebook-github-bot