Skip to content

[Flight] Optimize Retention of Weak Promises Abit #33736

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

Merged
merged 2 commits into from
Jul 9, 2025

Conversation

sebmarkbage
Copy link
Collaborator

We don't really need to retain a reference to whatever Promise another Promise was created in. Only awaits need to retain both their trigger and their previous context.

@sebmarkbage sebmarkbage requested a review from eps1lon July 8, 2025 18:32
@github-actions github-actions bot added the React Core Team Opened by a member of the React Core Team label Jul 8, 2025
@react-sizebot
Copy link

Comparing: a7a1165...8f33f76

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.68 kB 6.68 kB = 1.83 kB 1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 530.70 kB 530.70 kB = 93.70 kB 93.70 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB = 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 655.25 kB 655.25 kB = 115.40 kB 115.40 kB
facebook-www/ReactDOM-prod.classic.js = 675.13 kB 675.13 kB = 118.75 kB 118.75 kB
facebook-www/ReactDOM-prod.modern.js = 665.56 kB 665.56 kB = 117.12 kB 117.12 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-server-dom-esm/cjs/react-server-dom-esm-server.node.development.js +0.34% 196.71 kB 197.38 kB +0.31% 36.28 kB 36.39 kB
oss-experimental/react-server-dom-parcel/cjs/react-server-dom-parcel-server.node.development.js +0.33% 202.67 kB 203.34 kB +0.29% 36.96 kB 37.07 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.unbundled.development.js +0.32% 209.44 kB 210.11 kB +0.25% 38.12 kB 38.21 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.node.development.js +0.32% 210.59 kB 211.26 kB +0.25% 38.41 kB 38.50 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.development.js +0.32% 210.65 kB 211.32 kB +0.24% 38.42 kB 38.51 kB

Generated by 🚫 dangerJS against 8f33f76

@sebmarkbage sebmarkbage merged commit 49ded1d into facebook:main Jul 9, 2025
474 of 475 checks passed
sebmarkbage added a commit that referenced this pull request Jul 9, 2025
Follow up to #33736.

If we need to save on CPU/memory pressure, we can instead just pray and
hope that a Promise doesn't get garbage collected before we need to read
it.

This can cause fragile access to the Promise value in devtools
especially if it's a slow and pressured render.

Basically, you'd have to hope that GC doesn't run after the inner await
finishes its microtask callback and before the resolution of the
component being rendered is invoked.
unstubbable added a commit to vercel/next.js that referenced this pull request Jul 10, 2025
When running the following test isolated, and not as part of the full
test suite, the owner stacks for async I/O errors were missing the
top-most stack frame that's pointing at the `fetch` call.

```
pnpm test-dev test/e2e/app-dir/dynamic-io-errors/dynamic-io-errors.test.ts -t "should show a collapsed redbox with two errors"
```

The same could be reproduced with `pnpm next dev test/e2e/app-dir/dynamic-io-errors/fixtures/default`
at http://localhost:3000/dynamic-root, unless
http://localhost:3000/static was visited first.

The likely reason for that is that React's async I/O tracking was
recently optimized for performance reasons in facebook/react#33736 and
facebook/react#33737.

We can help React a bit with the tracking by explicitly awaiting our
`makeHangingPromise` calls. With this fix, the case from above reliably
works in isolation.
unstubbable added a commit to vercel/next.js that referenced this pull request Jul 10, 2025
When running the following test isolated, and not as part of the full
test suite, the owner stacks for async I/O errors were missing the
top-most stack frame that's pointing at the `fetch` call.

```
pnpm test-dev test/e2e/app-dir/dynamic-io-errors/dynamic-io-errors.test.ts -t "should show a collapsed redbox with two errors"
```

The same could be reproduced with `pnpm next dev test/e2e/app-dir/dynamic-io-errors/fixtures/default`
at http://localhost:3000/dynamic-root, unless
http://localhost:3000/static was visited first.

The likely reason for that is that React's async I/O tracking was
recently optimized for performance reasons in facebook/react#33736 and
facebook/react#33737.

We can help React a bit with the tracking by explicitly awaiting our
`makeHangingPromise` calls. With this fix, the case from above reliably
works in isolation.
unstubbable added a commit to vercel/next.js that referenced this pull request Jul 14, 2025
When running the following test isolated, and not as part of the full
test suite, the owner stacks for async I/O errors were missing the
top-most stack frame that's pointing at the `fetch` call.

```
pnpm test-dev test/e2e/app-dir/dynamic-io-errors/dynamic-io-errors.test.ts -t "should show a collapsed redbox with two errors"
```

The same could be reproduced with `pnpm next dev test/e2e/app-dir/dynamic-io-errors/fixtures/default`
at http://localhost:3000/dynamic-root, unless
http://localhost:3000/static was visited first.

The likely reason for that is that React's async I/O tracking was
recently optimized for performance reasons in facebook/react#33736 and
facebook/react#33737.

We can help React a bit with the tracking by explicitly awaiting our
`makeHangingPromise` calls. With this fix, the case from above reliably
works in isolation.
unstubbable added a commit to vercel/next.js that referenced this pull request Jul 15, 2025
When running the following test isolated, and not as part of the full
test suite, the owner stacks for async I/O errors were missing the
top-most stack frame that's pointing at the `fetch` call.

```
pnpm test-dev test/e2e/app-dir/dynamic-io-errors/dynamic-io-errors.test.ts -t "should show a collapsed redbox with two errors"
```

The same could be reproduced with `pnpm next dev test/e2e/app-dir/dynamic-io-errors/fixtures/default`
at http://localhost:3000/dynamic-root, unless
http://localhost:3000/static was visited first.

The likely reason for that is that React's async I/O tracking was
recently optimized for performance reasons in facebook/react#33736 and
facebook/react#33737.

We can help React a bit with the tracking by explicitly awaiting our
`makeHangingPromise` calls. With this fix, the case from above reliably
works in isolation.
unstubbable added a commit to vercel/next.js that referenced this pull request Jul 15, 2025
When running the following test isolated, and not as part of the full
test suite, the owner stacks for async I/O errors were missing the
top-most stack frame that's pointing at the `fetch` call.

```
pnpm test-dev test/e2e/app-dir/dynamic-io-errors/dynamic-io-errors.test.ts -t "should show a collapsed redbox with two errors"
```

The same could be reproduced with `pnpm next dev
test/e2e/app-dir/dynamic-io-errors/fixtures/default` at
http://localhost:3000/dynamic-root, unless http://localhost:3000/static
was visited first.

The likely reason for that is that React's async I/O tracking was
recently optimized for performance reasons in facebook/react#33736 and
facebook/react#33737.

We can help React a bit with the tracking by explicitly awaiting our
`makeHangingPromise` calls. With this fix, the case from above reliably
works in isolation.
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