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.

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
+63 −3

Conversation

@bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Sep 30, 2021 •
edited

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
bvaughn added 2 commits Sep 30, 2021
@bvaughn
Updated tests to support newer V8 stack formats
ac9e364
@bvaughn
Updated describeNativeComponentFrame() to support displayName in stac…
dabc7fd 
…k frames for newer versions of v8
@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 Sep 30, 2021
@sizebot
Copy link

@sizebot 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 Sep 30, 2021
@bvaughn bvaughn mentioned this pull request Sep 30, 2021
Open
akgupta0777
akgupta0777 reviewed Oct 1, 2021
View changes
Copy link
Contributor

@akgupta0777 akgupta0777 left a comment

Seems good.

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

@jstejada jstejada left a comment

looks good, thanks!

@bvaughn bvaughn merged commit c16b005 into facebook:main Oct 11, 2021
33 checks passed
@bvaughn bvaughn deleted the support-v8-error-format-changes branch Oct 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akgupta0777 akgupta0777

@jstejada jstejada

@sebmarkbage sebmarkbage

@rickhanlonii rickhanlonii

@acdlite acdlite

@josephsavona josephsavona

@salazarm salazarm

@gaearon gaearon

Assignees
No one assigned
Labels
CLA Signed React Core Team
Projects
None yet
Milestone
No milestone
Linked issues

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