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

Annotate legacy mode test in ReactDOMSingletonComponents #28339

Merged

Conversation

eps1lon
Copy link
Collaborator

@eps1lon eps1lon commented Feb 14, 2024

These are mainly regression tests for legacy root.

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Feb 14, 2024
@@ -996,17 +996,17 @@ describe('ReactDOM HostSingleton', () => {
});

// https://github.com/facebook/react/issues/26128
it('(#26128) does not throw when rendering at body', async () => {
it('(#26128) does not throw when rendering at body in legacy mode', async () => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We warn for this in the new root:

it('warns if creating a root on the document.body', async () => {

@react-sizebot
Copy link

Comparing: adadb81...f7c6f22

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.83 kB 176.83 kB = 55.11 kB 55.11 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 178.83 kB 178.83 kB = 55.69 kB 55.69 kB
facebook-www/ReactDOM-prod.classic.js = 592.18 kB 592.18 kB = 104.43 kB 104.43 kB
facebook-www/ReactDOM-prod.modern.js = 575.46 kB 575.46 kB = 101.43 kB 101.43 kB
test_utils/ReactAllWarnings.js Deleted 66.26 kB 0.00 kB Deleted 16.32 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.26 kB 0.00 kB Deleted 16.32 kB 0.00 kB

Generated by 🚫 dangerJS against f7c6f22

ReactDOM.render(<body />, document.documentElement);
});

// https://github.com/facebook/react/issues/26128
it('(#26128) does not throw when rendering at document', async () => {
it('(#26128) does not throw when rendering at document in legacy mode', async () => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We disallow this at the type level but have no runtime warning

ReactDOM.render(<div />, document.body);
});

// https://github.com/facebook/react/issues/26128
it('(#26128) does not throw when rendering at <html>', async () => {
it('(#26128) does not throw when rendering at <html> in legacy mode', async () => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As far as I can tell we only have tests for hydrating into <html>

@eps1lon eps1lon marked this pull request as ready for review February 14, 2024 20:27
Copy link
Contributor

@jackpope jackpope left a comment

Choose a reason for hiding this comment

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

Maybe consider moving these tests into a "legacy" version of the test module to make removal easier in the future. But this annotation is helpful and lgtm

@eps1lon eps1lon merged commit 62a9c7d into facebook:main Feb 17, 2024
36 checks passed
@eps1lon eps1lon deleted the test/create-root/ReactDOMSingletonComponents branch February 18, 2024 15:54
huozhi added a commit to vercel/next.js that referenced this pull request Feb 23, 2024
### React upstream changes

- facebook/react#28333
- facebook/react#28334
- facebook/react#28378
- facebook/react#28377
- facebook/react#28376
- facebook/react#28338
- facebook/react#28331
- facebook/react#28336
- facebook/react#28320
- facebook/react#28317
- facebook/react#28375
- facebook/react#28367
- facebook/react#28380
- facebook/react#28368
- facebook/react#28343
- facebook/react#28355
- facebook/react#28374
- facebook/react#28362
- facebook/react#28344
- facebook/react#28339
- facebook/react#28353
- facebook/react#28346
- facebook/react#25790
- facebook/react#28352
- facebook/react#28326
- facebook/react#27688
- facebook/react#28329
- facebook/react#28332
- facebook/react#28340
- facebook/react#28327
- facebook/react#28325
- facebook/react#28324
- facebook/react#28309
- facebook/react#28310
- facebook/react#28307
- facebook/react#28306
- facebook/react#28315
- facebook/react#28318
- facebook/react#28226
- facebook/react#28308
- facebook/react#27563
- facebook/react#28297
- facebook/react#28286
- facebook/react#28284
- facebook/react#28275
- facebook/react#28145
- facebook/react#28301
- facebook/react#28224
- facebook/react#28152
- facebook/react#28296
- facebook/react#28294
- facebook/react#28279
- facebook/react#28273
- facebook/react#28269
- facebook/react#28376
- facebook/react#28338
- facebook/react#28331
- facebook/react#28336
- facebook/react#28320
- facebook/react#28317
- facebook/react#28375
- facebook/react#28367
- facebook/react#28380
- facebook/react#28368
- facebook/react#28343
- facebook/react#28355
- facebook/react#28374
- facebook/react#28362
- facebook/react#28344
- facebook/react#28339
- facebook/react#28353
- facebook/react#28346
- facebook/react#25790
- facebook/react#28352
- facebook/react#28326
- facebook/react#27688
- facebook/react#28329
- facebook/react#28332
- facebook/react#28340
- facebook/react#28327
- facebook/react#28325
- facebook/react#28324
- facebook/react#28309
- facebook/react#28310
- facebook/react#28307
- facebook/react#28306
- facebook/react#28315
- facebook/react#28318
- facebook/react#28226
- facebook/react#28308
- facebook/react#27563
- facebook/react#28297
- facebook/react#28286
- facebook/react#28284
- facebook/react#28275
- facebook/react#28145
- facebook/react#28301
- facebook/react#28224
- facebook/react#28152
- facebook/react#28296
- facebook/react#28294
- facebook/react#28279
- facebook/react#28273
- facebook/react#28269

Closes NEXT-2542


Disable ppr test for strict mode for now, @acdlite will check it and
we'll sync again
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
)

These are mainly regression tests for legacy root.
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
These are mainly regression tests for legacy root.

DiffTrain build for commit 62a9c7d.
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.

None yet

4 participants