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

Add a test for issue #28595 #28601

Closed
wants to merge 1 commit into from

Conversation

unstubbable
Copy link
Contributor

@unstubbable unstubbable commented Mar 20, 2024

The added test, intended to fail and reproduce the reported issue, unexpectedly passes in its current state. I see three possible reasons:

  1. The bug report could be invalid.
  2. How I've structured the test might be insufficient to replicate what ai/rsc is doing.
  3. Something in the test setup could be masking the actual error. (Maybe related to fake timers?)

If the problem lies in reason 2 or 3, this test could possibly serve as a foundation for further investigation.

@react-sizebot
Copy link

react-sizebot commented Mar 20, 2024

Comparing: a493901...d5cbcd6

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.min.js = 176.84 kB 176.84 kB = 54.92 kB 54.92 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 173.27 kB 173.27 kB = 54.04 kB 54.04 kB
facebook-www/ReactDOM-prod.classic.js = 594.30 kB 594.30 kB = 104.45 kB 104.45 kB
facebook-www/ReactDOM-prod.modern.js = 577.56 kB 577.56 kB = 101.48 kB 101.48 kB
test_utils/ReactAllWarnings.js Deleted 66.48 kB 0.00 kB Deleted 16.26 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 66.48 kB 0.00 kB Deleted 16.26 kB 0.00 kB

Generated by 🚫 dangerJS against d5cbcd6

@unstubbable
Copy link
Contributor Author

unstubbable commented Mar 20, 2024

Oh cool, the test is indeed failing in the CI. Not sure why it succeeds on my machine though. 🤔 (see comment below)

The added test, intended to fail and reproduce the [reported
issue](facebook#28595), unexpectedly
passes in its current state. I see three possible reasons:

1. The bug report could be invalid.
2. How I've structured the test might be insufficient to replicate what
   `ai/rsc` is doing.
3. Something in the test setup could be masking the actual error.
   (Maybe related to fake timers?)

If the problem lies in reason 2 or 3, this test could possibly serve as
a foundation for further investigation.
@unstubbable
Copy link
Contributor Author

unstubbable commented Mar 23, 2024

Duh, I need to run the tests with -r=stable, because only those failed.

@unstubbable
Copy link
Contributor Author

superseded by #28620

@unstubbable unstubbable deleted the lazy-node-test branch March 23, 2024 16:52
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.

3 participants