Skip to content

Conversation

@unstubbable
Copy link
Collaborator

@unstubbable unstubbable commented May 21, 2024

Triggered by vercel/next.js#66033

I was suspecting that the bug was introduced with #28996, but I could not make the test succeed on a commit before that PR, so maybe this assumption (or the reproduction) is wrong.
EDIT: With a revert of #28996, the test does succeed.

Triggered by vercel/next.js#66033

I was suspecting that the bug was introduced with facebook#28996, but I could not make the test succeed on a commit before that PR, so maybe this assumption is wrong.
@vercel
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 5:02pm

@react-sizebot
Copy link

Comparing: 5cc9f69...00af602

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 = 1.82 kB 1.82 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 495.71 kB 495.71 kB = 88.79 kB 88.78 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.67 kB 6.67 kB = 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
test_utils/ReactAllWarnings.js Deleted 64.35 kB 0.00 kB Deleted 16.05 kB 0.00 kB

Generated by 🚫 dangerJS against 00af602

expect(container.innerHTML).toBe('<span>Hello, World!</span>');
});

it('should resolve deduped objects within the same model root when it is blocked', async () => {
Copy link
Contributor

@lubieowoce lubieowoce May 21, 2024

Choose a reason for hiding this comment

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

maybe do

Suggested change
it('should resolve deduped objects within the same model root when it is blocked', async () => {
it.failing('should resolve deduped objects within the same model root when it is blocked', async () => {

so that it passes CI?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried that already in #29129, but the Jest setup in the React repo does not seem to support this.

sebmarkbage added a commit that referenced this pull request May 21, 2024
…locked state (#29201)

Fixes #29200

The cyclic state might have added listeners that will still need to be
invoked. This happens if we have a cyclic reference AND end up blocked.

We have already cleared these before entering the parsing when we enter
the CYCLIC state so we they already have the right type. If listeners
are added during this phase they should carry over to the blocked state.

---------

Co-authored-by: Hendrik Liebau <[email protected]>
@unstubbable unstubbable deleted the failed-test-resolve-deduped-when-blocked branch May 21, 2024 18:13
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.

4 participants