Skip to content

[Flight] Yolo Retention of Promises #33737

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 1 commit into from
Jul 9, 2025

Conversation

sebmarkbage
Copy link
Collaborator

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.

@sebmarkbage sebmarkbage requested a review from eps1lon July 8, 2025 18:37
@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

react-sizebot commented Jul 8, 2025

Comparing: e6dc25d...08ac8be

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 +0.05% 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.11 kB 117.11 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-webpack/cjs/react-server-dom-webpack-server.node.development.js = 213.93 kB 212.95 kB = 38.83 kB 38.71 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.node.development.js = 213.88 kB 212.89 kB = 38.82 kB 38.70 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.unbundled.development.js = 212.73 kB 211.74 kB = 38.54 kB 38.42 kB
oss-experimental/react-server-dom-parcel/cjs/react-server-dom-parcel-server.node.development.js = 205.95 kB 204.97 kB = 37.40 kB 37.26 kB
oss-experimental/react-server-dom-esm/cjs/react-server-dom-esm-server.node.development.js = 200.06 kB 199.08 kB = 36.75 kB 36.61 kB

Generated by 🚫 dangerJS against 08ac8be

@sebmarkbage
Copy link
Collaborator Author

I think we're still missing one connection anyway between a new Promise and what ultimately resolved it so I'll just land this for now for speed.

@sebmarkbage sebmarkbage merged commit 033edca into facebook:main Jul 9, 2025
468 of 469 checks passed
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