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

[Fizz] Align recoverable error serialization in dev mode #28340

Merged
merged 2 commits into from
Feb 15, 2024

Conversation

sebmarkbage
Copy link
Collaborator

Same as #28327 but for Fizz.

One thing that's weird about this recoverable error is that we don't send the regular stack for it, just the component stack it seems. This is missing some potential information and if we move toward integrated since stacks it would be one thing.

@sebmarkbage sebmarkbage requested review from gaearon and gnoff February 14, 2024 23:24
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Feb 14, 2024
@@ -33,6 +33,7 @@ import type {ComponentStackNode} from './ReactFizzComponentStack';
import type {TreeContext} from './ReactFizzTreeContext';
import type {ThenableState} from './ReactFizzThenable';
import {enableRenderableContext} from 'shared/ReactFeatureFlags';
import {describeObjectForErrorMessage} from 'shared/ReactSerializationErrors';
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fizz is typically not a "serializer", except in this one case where we serialize errors it is.

@react-sizebot
Copy link

react-sizebot commented Feb 14, 2024

Comparing: a7144f2...685f199

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 = 176.83 kB 176.83 kB = 55.11 kB 55.11 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 178.83 kB 178.83 kB = 55.69 kB 55.69 kB
facebook-www/ReactDOM-prod.classic.js = 592.18 kB 592.18 kB = 104.43 kB 104.43 kB
facebook-www/ReactDOM-prod.modern.js = 575.46 kB 575.46 kB = 101.43 kB 101.43 kB
oss-stable-semver/react-server/cjs/react-server.development.js +3.24% 198.17 kB 204.60 kB +2.59% 46.80 kB 48.01 kB
oss-stable/react-server/cjs/react-server.development.js +3.24% 198.17 kB 204.60 kB +2.59% 46.80 kB 48.01 kB
oss-experimental/react-server/cjs/react-server.development.js +3.07% 209.63 kB 216.06 kB +2.48% 48.72 kB 49.93 kB
test_utils/ReactAllWarnings.js Deleted 66.26 kB 0.00 kB Deleted 16.32 kB 0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable-semver/react-server/cjs/react-server.development.js +3.24% 198.17 kB 204.60 kB +2.59% 46.80 kB 48.01 kB
oss-stable/react-server/cjs/react-server.development.js +3.24% 198.17 kB 204.60 kB +2.59% 46.80 kB 48.01 kB
oss-experimental/react-server/cjs/react-server.development.js +3.07% 209.63 kB 216.06 kB +2.48% 48.72 kB 49.93 kB
oss-stable-semver/react-dom/umd/react-dom-server.browser.development.js +1.56% 439.47 kB 446.30 kB +1.47% 95.91 kB 97.33 kB
oss-stable/react-dom/umd/react-dom-server.browser.development.js +1.56% 439.49 kB 446.33 kB +1.47% 95.94 kB 97.35 kB
oss-stable-semver/react-dom/umd/react-dom-server-legacy.browser.development.js +1.55% 439.94 kB 446.77 kB +1.48% 95.73 kB 97.15 kB
oss-stable/react-dom/umd/react-dom-server-legacy.browser.development.js +1.55% 439.97 kB 446.80 kB +1.48% 95.75 kB 97.17 kB
oss-stable-semver/react-dom/cjs/react-dom-server.bun.development.js +1.55% 414.07 kB 420.49 kB +1.55% 92.79 kB 94.23 kB
oss-stable/react-dom/cjs/react-dom-server.bun.development.js +1.55% 414.09 kB 420.52 kB +1.55% 92.81 kB 94.25 kB
facebook-www/ReactDOMServerStreaming-dev.modern.js +1.55% 475.77 kB 483.15 kB +1.52% 96.61 kB 98.08 kB
oss-stable-semver/react-dom/cjs/react-dom-server.node.development.js +1.53% 418.87 kB 425.30 kB +1.48% 93.87 kB 95.26 kB
oss-stable/react-dom/cjs/react-dom-server.node.development.js +1.53% 418.90 kB 425.33 kB +1.48% 93.90 kB 95.28 kB
oss-stable-semver/react-dom/cjs/react-dom-server.browser.development.js +1.53% 419.91 kB 426.34 kB +1.50% 94.93 kB 96.35 kB
oss-stable/react-dom/cjs/react-dom-server.browser.development.js +1.53% 419.94 kB 426.36 kB +1.50% 94.95 kB 96.37 kB
oss-stable-semver/react-dom/cjs/react-dom-server-legacy.browser.development.js +1.53% 420.37 kB 426.80 kB +1.48% 94.75 kB 96.16 kB
oss-stable/react-dom/cjs/react-dom-server-legacy.browser.development.js +1.53% 420.40 kB 426.82 kB +1.49% 94.77 kB 96.18 kB
oss-stable-semver/react-dom/cjs/react-dom-server.edge.development.js +1.53% 420.50 kB 426.92 kB +1.49% 95.07 kB 96.48 kB
oss-stable/react-dom/cjs/react-dom-server.edge.development.js +1.53% 420.52 kB 426.95 kB +1.49% 95.09 kB 96.51 kB
facebook-www/ReactDOMServer-dev.modern.js +1.53% 483.00 kB 490.38 kB +1.48% 98.22 kB 99.67 kB
oss-stable-semver/react-dom/cjs/react-dom-server-legacy.node.development.js +1.52% 422.23 kB 428.65 kB +1.47% 95.22 kB 96.62 kB
oss-stable/react-dom/cjs/react-dom-server-legacy.node.development.js +1.52% 422.25 kB 428.68 kB +1.48% 95.24 kB 96.65 kB
oss-experimental/react-dom/umd/react-dom-server-legacy.browser.development.js +1.50% 454.64 kB 461.48 kB +1.44% 98.05 kB 99.46 kB
facebook-www/ReactDOMServer-dev.classic.js +1.50% 491.32 kB 498.70 kB +1.48% 99.86 kB 101.34 kB
oss-experimental/react-dom/cjs/react-dom-server.bun.development.js +1.50% 428.18 kB 434.61 kB +1.50% 95.14 kB 96.57 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.browser.development.js +1.48% 434.49 kB 440.91 kB +1.44% 97.10 kB 98.50 kB
oss-experimental/react-dom/umd/react-dom-server.browser.development.js +1.47% 463.92 kB 470.75 kB +1.41% 99.22 kB 100.62 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.node.development.js +1.47% 436.34 kB 442.77 kB +1.43% 97.57 kB 98.97 kB
oss-experimental/react-dom/cjs/react-dom-server.node.development.js +1.45% 441.79 kB 448.22 kB +1.43% 97.43 kB 98.83 kB
oss-experimental/react-dom/cjs/react-dom-server.browser.development.js +1.45% 443.36 kB 449.79 kB +1.44% 98.21 kB 99.62 kB
oss-experimental/react-dom/cjs/react-dom-server.edge.development.js +1.45% 443.94 kB 450.37 kB +1.44% 98.35 kB 99.76 kB
test_utils/ReactAllWarnings.js Deleted 66.26 kB 0.00 kB Deleted 16.32 kB 0.00 kB

Generated by 🚫 dangerJS against 685f199

@sebmarkbage sebmarkbage force-pushed the fizzobjecterror branch 3 times, most recently from bfabf71 to aa9f1e7 Compare February 15, 2024 00:50
These don't really make sense to add to the message string since these prefixes
are typically printed when the errors are printed anyway and are part of .stack.

So prefixing just leads to duplicate prefixes.
@sebmarkbage sebmarkbage merged commit 2e470a7 into facebook:main Feb 15, 2024
36 checks passed
github-actions bot pushed a commit that referenced this pull request Feb 15, 2024
Same as #28327 but for Fizz.

One thing that's weird about this recoverable error is that we don't
send the regular stack for it, just the component stack it seems. This
is missing some potential information and if we move toward integrated
since stacks it would be one thing.

DiffTrain build for [2e470a7](2e470a7)
huozhi added a commit to vercel/next.js that referenced this pull request Feb 23, 2024
### React upstream changes

- facebook/react#28333
- facebook/react#28334
- facebook/react#28378
- facebook/react#28377
- facebook/react#28376
- facebook/react#28338
- facebook/react#28331
- facebook/react#28336
- facebook/react#28320
- facebook/react#28317
- facebook/react#28375
- facebook/react#28367
- facebook/react#28380
- facebook/react#28368
- facebook/react#28343
- facebook/react#28355
- facebook/react#28374
- facebook/react#28362
- facebook/react#28344
- facebook/react#28339
- facebook/react#28353
- facebook/react#28346
- facebook/react#25790
- facebook/react#28352
- facebook/react#28326
- facebook/react#27688
- facebook/react#28329
- facebook/react#28332
- facebook/react#28340
- facebook/react#28327
- facebook/react#28325
- facebook/react#28324
- facebook/react#28309
- facebook/react#28310
- facebook/react#28307
- facebook/react#28306
- facebook/react#28315
- facebook/react#28318
- facebook/react#28226
- facebook/react#28308
- facebook/react#27563
- facebook/react#28297
- facebook/react#28286
- facebook/react#28284
- facebook/react#28275
- facebook/react#28145
- facebook/react#28301
- facebook/react#28224
- facebook/react#28152
- facebook/react#28296
- facebook/react#28294
- facebook/react#28279
- facebook/react#28273
- facebook/react#28269
- facebook/react#28376
- facebook/react#28338
- facebook/react#28331
- facebook/react#28336
- facebook/react#28320
- facebook/react#28317
- facebook/react#28375
- facebook/react#28367
- facebook/react#28380
- facebook/react#28368
- facebook/react#28343
- facebook/react#28355
- facebook/react#28374
- facebook/react#28362
- facebook/react#28344
- facebook/react#28339
- facebook/react#28353
- facebook/react#28346
- facebook/react#25790
- facebook/react#28352
- facebook/react#28326
- facebook/react#27688
- facebook/react#28329
- facebook/react#28332
- facebook/react#28340
- facebook/react#28327
- facebook/react#28325
- facebook/react#28324
- facebook/react#28309
- facebook/react#28310
- facebook/react#28307
- facebook/react#28306
- facebook/react#28315
- facebook/react#28318
- facebook/react#28226
- facebook/react#28308
- facebook/react#27563
- facebook/react#28297
- facebook/react#28286
- facebook/react#28284
- facebook/react#28275
- facebook/react#28145
- facebook/react#28301
- facebook/react#28224
- facebook/react#28152
- facebook/react#28296
- facebook/react#28294
- facebook/react#28279
- facebook/react#28273
- facebook/react#28269

Closes NEXT-2542


Disable ppr test for strict mode for now, @acdlite will check it and
we'll sync again
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
)

Same as facebook#28327 but for Fizz.

One thing that's weird about this recoverable error is that we don't
send the regular stack for it, just the component stack it seems. This
is missing some potential information and if we move toward integrated
since stacks it would be one thing.
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
Same as #28327 but for Fizz.

One thing that's weird about this recoverable error is that we don't
send the regular stack for it, just the component stack it seems. This
is missing some potential information and if we move toward integrated
since stacks it would be one thing.

DiffTrain build for commit 2e470a7.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants