Skip to content

Conversation

@sebmarkbage
Copy link
Collaborator

I noticed we weren't deduping these cases.

@sebmarkbage sebmarkbage requested a review from unstubbable June 29, 2025 03:32
@github-actions github-actions bot added the React Core Team Opened by a member of the React Core Team label Jun 29, 2025
@react-sizebot
Copy link

Comparing: 811e203...074c5c0

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 +0.05% 1.83 kB 1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 530.57 kB 530.57 kB = 93.67 kB 93.67 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 = 651.66 kB 651.66 kB = 114.78 kB 114.78 kB
facebook-www/ReactDOM-prod.classic.js = 674.81 kB 674.81 kB = 118.78 kB 118.78 kB
facebook-www/ReactDOM-prod.modern.js = 665.30 kB 665.30 kB = 117.19 kB 117.19 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 074c5c0

@sebmarkbage sebmarkbage merged commit 9c2a8dd into facebook:main Jun 29, 2025
247 checks passed
sebmarkbage added a commit that referenced this pull request Jun 29, 2025
Stacked on #33666.

If we ever get a future reference to a cycle and that reference gets
eagerly parsed before the target has loaded then we can end up with a
cycle that never gets resolved. That's because our cycle resolution only
works if the cyclic future reference is created synchronously within the
parsing path of the child.

I haven't been able to construct a normal scenario where this would
break. So this doesn't fail any tests. However, I can construct it with
debug info since those are eagerly evaluated. It's also a prerequisite
if the debug data can come out of order, like if it's on a different
stream.

The fix here is to make all the internal dependencies in the "listener"
list into introspectable objects instead of closures. That way we can
traverse the list of dependencies of a blocked reference to see if it
ends up in a cycle and therefore skip the reference.

It would be nice to address this once and for all to be more resilient
to server changes, but I'm not sure if it's worth this complexity and
the extra CPU cost of tracing the dependencies. Especially if it's just
for debug data.

closes #32316
fixes vercel/next.js#72104

---------

Co-authored-by: Hendrik Liebau <mail@hendrik-liebau.de>
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