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

[Flight] don't overwrite existing chunk listeners in 'wakeChunkIfInitialized' #29204

Merged
merged 1 commit into from
May 21, 2024

Conversation

lubieowoce
Copy link
Contributor

@lubieowoce lubieowoce commented May 21, 2024

Follow-up to #29201. If a chunk had listeners attached already (e.g. because .then was called on the chunk returned from createFromReadableStream), wakeChunkIfInitialized would overwrite any listeners added during chunk initialization. This caused cyclic path references within that chunk to never resolve. Fixed by merging the two arrays of listeners.

Copy link

vercel bot commented May 21, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 21, 2024 8:48pm

@react-sizebot
Copy link

Comparing: 0a0a5c0...ab8f86b

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.js = 6.66 kB 6.66 kB +0.17% 1.82 kB 1.82 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 495.71 kB 495.71 kB = 88.78 kB 88.78 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.67 kB 6.67 kB +0.11% 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 500.51 kB 500.51 kB = 89.47 kB 89.47 kB
facebook-www/ReactDOM-prod.classic.js = 592.86 kB 592.86 kB = 104.28 kB 104.28 kB
facebook-www/ReactDOM-prod.modern.js = 569.08 kB 569.08 kB = 100.69 kB 100.69 kB
test_utils/ReactAllWarnings.js Deleted 64.35 kB 0.00 kB Deleted 16.05 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-dom-esm/cjs/react-server-dom-esm-client.browser.production.js +0.94% 43.78 kB 44.19 kB +0.75% 9.06 kB 9.12 kB
oss-stable/react-server-dom-esm/cjs/react-server-dom-esm-client.browser.production.js +0.94% 43.78 kB 44.19 kB +0.75% 9.06 kB 9.12 kB
oss-experimental/react-server-dom-esm/cjs/react-server-dom-esm-client.browser.production.js +0.93% 44.26 kB 44.68 kB +0.73% 9.13 kB 9.19 kB
oss-stable-semver/react-client/cjs/react-client-flight.production.js +0.93% 46.81 kB 47.24 kB +0.84% 9.05 kB 9.12 kB
oss-stable/react-client/cjs/react-client-flight.production.js +0.93% 46.81 kB 47.24 kB +0.84% 9.05 kB 9.12 kB
oss-experimental/react-client/cjs/react-client-flight.production.js +0.92% 47.21 kB 47.64 kB +0.79% 9.10 kB 9.17 kB
oss-stable-semver/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.browser.production.js +0.91% 45.32 kB 45.73 kB +0.71% 9.41 kB 9.48 kB
oss-stable/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.browser.production.js +0.91% 45.32 kB 45.73 kB +0.71% 9.41 kB 9.48 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.production.js +0.90% 45.66 kB 46.07 kB +0.73% 9.50 kB 9.57 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.production.js +0.90% 45.66 kB 46.07 kB +0.73% 9.50 kB 9.57 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.browser.production.js +0.90% 45.81 kB 46.22 kB +0.71% 9.48 kB 9.55 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.production.js +0.89% 46.14 kB 46.55 kB +0.72% 9.57 kB 9.64 kB
oss-stable-semver/react-server-dom-esm/cjs/react-server-dom-esm-client.node.production.js +0.85% 48.16 kB 48.57 kB +0.73% 10.09 kB 10.17 kB
oss-stable/react-server-dom-esm/cjs/react-server-dom-esm-client.node.production.js +0.85% 48.16 kB 48.57 kB +0.73% 10.09 kB 10.17 kB
oss-experimental/react-server-dom-esm/cjs/react-server-dom-esm-client.node.production.js +0.84% 48.64 kB 49.05 kB +0.71% 10.17 kB 10.24 kB
oss-stable-semver/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.node.unbundled.production.js +0.83% 49.41 kB 49.82 kB +0.71% 10.38 kB 10.45 kB
oss-stable/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.node.unbundled.production.js +0.83% 49.41 kB 49.82 kB +0.71% 10.38 kB 10.45 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-client.node.unbundled.production.js +0.83% 49.41 kB 49.82 kB +0.71% 10.38 kB 10.45 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-client.node.unbundled.production.js +0.83% 49.41 kB 49.82 kB +0.71% 10.38 kB 10.45 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.node.unbundled.production.js +0.82% 49.89 kB 50.31 kB +0.72% 10.44 kB 10.52 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-client.node.unbundled.production.js +0.82% 49.90 kB 50.31 kB +0.71% 10.45 kB 10.52 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-client.node.production.js +0.82% 50.09 kB 50.50 kB +0.76% 10.54 kB 10.62 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-client.node.production.js +0.82% 50.09 kB 50.50 kB +0.76% 10.54 kB 10.62 kB
oss-stable-semver/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.node.production.js +0.82% 50.10 kB 50.51 kB +0.76% 10.54 kB 10.62 kB
oss-stable/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.node.production.js +0.82% 50.10 kB 50.51 kB +0.76% 10.54 kB 10.62 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-client.node.production.js +0.81% 50.57 kB 50.98 kB +0.90% 10.61 kB 10.71 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.node.production.js +0.81% 50.58 kB 51.00 kB +0.90% 10.61 kB 10.70 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-client.edge.production.js +0.80% 51.13 kB 51.54 kB +0.69% 10.74 kB 10.81 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-client.edge.production.js +0.80% 51.13 kB 51.54 kB +0.69% 10.74 kB 10.81 kB
oss-stable-semver/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.edge.production.js +0.80% 51.14 kB 51.55 kB +0.70% 10.73 kB 10.81 kB
oss-stable/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.edge.production.js +0.80% 51.14 kB 51.55 kB +0.70% 10.73 kB 10.81 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-client.edge.production.js +0.80% 51.62 kB 52.03 kB +0.65% 10.81 kB 10.88 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.edge.production.js +0.80% 51.63 kB 52.04 kB +0.66% 10.80 kB 10.87 kB
oss-stable-semver/react-server-dom-esm/esm/react-server-dom-esm-client.browser.production.js +0.53% 72.79 kB 73.18 kB +0.36% 15.30 kB 15.36 kB
oss-stable/react-server-dom-esm/esm/react-server-dom-esm-client.browser.production.js +0.53% 72.79 kB 73.18 kB +0.36% 15.30 kB 15.36 kB
oss-experimental/react-server-dom-esm/esm/react-server-dom-esm-client.browser.production.js +0.53% 73.53 kB 73.92 kB +0.36% 15.41 kB 15.47 kB
oss-stable-semver/react-client/cjs/react-client-flight.development.js +0.44% 89.90 kB 90.29 kB +0.36% 19.95 kB 20.02 kB
oss-stable/react-client/cjs/react-client-flight.development.js +0.44% 89.90 kB 90.29 kB +0.36% 19.95 kB 20.02 kB
oss-stable-semver/react-server-dom-esm/esm/react-server-dom-esm-client.browser.development.js +0.44% 89.91 kB 90.30 kB +0.37% 19.66 kB 19.73 kB
oss-stable/react-server-dom-esm/esm/react-server-dom-esm-client.browser.development.js +0.44% 89.91 kB 90.30 kB +0.37% 19.66 kB 19.73 kB
oss-stable-semver/react-server-dom-esm/cjs/react-server-dom-esm-client.browser.development.js +0.44% 90.15 kB 90.54 kB +0.37% 19.73 kB 19.80 kB
oss-stable/react-server-dom-esm/cjs/react-server-dom-esm-client.browser.development.js +0.44% 90.15 kB 90.54 kB +0.37% 19.73 kB 19.80 kB
oss-experimental/react-client/cjs/react-client-flight.development.js +0.43% 91.06 kB 91.45 kB +0.31% 20.15 kB 20.21 kB
oss-experimental/react-server-dom-esm/esm/react-server-dom-esm-client.browser.development.js +0.43% 91.07 kB 91.46 kB +0.35% 19.86 kB 19.92 kB
oss-experimental/react-server-dom-esm/cjs/react-server-dom-esm-client.browser.development.js +0.43% 91.31 kB 91.70 kB +0.35% 19.92 kB 19.99 kB
oss-stable-semver/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.browser.development.js +0.42% 93.40 kB 93.79 kB +0.35% 20.70 kB 20.77 kB
oss-stable/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.browser.development.js +0.42% 93.40 kB 93.79 kB +0.35% 20.70 kB 20.77 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.development.js +0.42% 93.91 kB 94.30 kB +0.35% 20.87 kB 20.94 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.development.js +0.42% 93.91 kB 94.30 kB +0.35% 20.87 kB 20.94 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.browser.development.js +0.42% 94.56 kB 94.95 kB +0.33% 20.90 kB 20.97 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.development.js +0.42% 95.07 kB 95.46 kB +0.33% 21.07 kB 21.14 kB
oss-stable-semver/react-server-dom-esm/cjs/react-server-dom-esm-client.node.development.js +0.40% 97.82 kB 98.22 kB +0.30% 21.74 kB 21.81 kB
oss-stable/react-server-dom-esm/cjs/react-server-dom-esm-client.node.development.js +0.40% 97.82 kB 98.22 kB +0.30% 21.74 kB 21.81 kB
oss-experimental/react-server-dom-esm/cjs/react-server-dom-esm-client.node.development.js +0.40% 98.98 kB 99.38 kB +0.27% 21.94 kB 22.00 kB
oss-stable-semver/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.node.unbundled.development.js +0.40% 99.60 kB 100.00 kB +0.29% 22.31 kB 22.37 kB
oss-stable/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.node.unbundled.development.js +0.40% 99.60 kB 100.00 kB +0.29% 22.31 kB 22.37 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-client.node.unbundled.development.js +0.40% 99.64 kB 100.03 kB +0.29% 22.33 kB 22.40 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-client.node.unbundled.development.js +0.40% 99.64 kB 100.03 kB +0.29% 22.33 kB 22.40 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.node.unbundled.development.js +0.39% 100.76 kB 101.16 kB +0.27% 22.50 kB 22.56 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-client.node.unbundled.development.js +0.39% 100.80 kB 101.19 kB +0.27% 22.53 kB 22.59 kB
oss-stable-semver/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.node.development.js +0.39% 101.04 kB 101.43 kB +0.29% 22.70 kB 22.76 kB
oss-stable/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.node.development.js +0.39% 101.04 kB 101.43 kB +0.29% 22.70 kB 22.76 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-client.node.development.js +0.39% 101.07 kB 101.46 kB +0.29% 22.74 kB 22.80 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-client.node.development.js +0.39% 101.07 kB 101.46 kB +0.29% 22.74 kB 22.80 kB
oss-stable-semver/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.edge.development.js +0.39% 102.12 kB 102.51 kB +0.28% 22.90 kB 22.97 kB
oss-stable/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.edge.development.js +0.39% 102.12 kB 102.51 kB +0.28% 22.90 kB 22.97 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-client.edge.development.js +0.39% 102.14 kB 102.54 kB +0.28% 22.94 kB 23.01 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-client.edge.development.js +0.39% 102.14 kB 102.54 kB +0.28% 22.94 kB 23.01 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.node.development.js +0.39% 102.20 kB 102.59 kB +0.26% 22.89 kB 22.95 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-client.node.development.js +0.39% 102.23 kB 102.62 kB +0.26% 22.93 kB 22.99 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.edge.development.js +0.38% 103.28 kB 103.67 kB +0.26% 23.10 kB 23.16 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-client.edge.development.js +0.38% 103.30 kB 103.70 kB +0.26% 23.14 kB 23.20 kB
test_utils/ReactAllWarnings.js Deleted 64.35 kB 0.00 kB Deleted 16.05 kB 0.00 kB

Generated by 🚫 dangerJS against ab8f86b

@lubieowoce lubieowoce changed the title don't overwrite existing chunk listeners in 'wakeChunkIfInitialized' [Flight] don't overwrite existing chunk listeners in 'wakeChunkIfInitialized' May 21, 2024
@lubieowoce
Copy link
Contributor Author

lubieowoce commented May 21, 2024

A bit of context:
https://github.com/lubieowoce/react/blob/ab8f86b30db84338717dcaa4a57242fb168e89ca/packages/react-client/src/ReactFlightClient.js#L467

The test added in #29201 didn't go into that if, because the chunk/promise was only then'd during render, so we didn't have any resolveListeners yet at that point. that's why we missed this.

@gnoff gnoff merged commit 9b3f909 into facebook:main May 21, 2024
40 checks passed
@sebmarkbage
Copy link
Collaborator

You have to change ReplyServer too whenever you change these.

@lubieowoce
Copy link
Contributor Author

lubieowoce commented May 21, 2024

@sebmarkbage yeah, sorry about that, unfortunately we only noticed that after we merged this. @gnoff said he's gonna add that in a bit (i'm AFK r/n)

gnoff added a commit to gnoff/react that referenced this pull request May 21, 2024
…isteners

In facebook#29201 a fix was made to ensure we don't "forget" about some listeners when handling cyclic chunks.
In facebook#29204 another fix was made for a special case when the chunk already has listeners before it first resolves.

This implements the followup fix for Flight Reply which was originally missed in facebook#29204

Co-Authored-by: Janka Uryga <lolzatu2@gmail.com>
gnoff added a commit that referenced this pull request May 21, 2024
…isteners (#29207)

In #29201 a fix was made to ensure we don't "forget" about some
listeners when handling cyclic chunks.
In #29204 another fix was made for a special case when the chunk already
has listeners before it first resolves.

This implements the followup fix for Flight Reply which was originally
missed in #29204

Co-authored-by: Janka Uryga <lolzatu2@gmail.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.

5 participants