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

Remove react-test-renderer/shallow export #28497

Merged
merged 2 commits into from
Mar 26, 2024

Conversation

jackpope
Copy link
Contributor

@jackpope jackpope commented Mar 5, 2024

Based on

Summary

The shallow renderer was extracted from the repo years ago and published by enzyme: https://github.com/enzymejs/react-shallow-renderer

We no longer need to reexport under the react-test-renderer namespace. People can import react-shallow-renderer as needed

How did you test this change?

  • Observe shallow.js in react-test-renderer package from standard build
  • Run build with changes on this branch
  • Observe no more shallow.js export in build output

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Mar 5, 2024
@react-sizebot
Copy link

react-sizebot commented Mar 5, 2024

Comparing: f73d11f...a4c5ff6

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 = 175.94 kB 175.94 kB = 54.54 kB 54.54 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 172.42 kB 172.42 kB = 53.72 kB 53.72 kB
facebook-www/ReactDOM-prod.classic.js = 590.84 kB 590.84 kB = 103.76 kB 103.76 kB
facebook-www/ReactDOM-prod.modern.js = 574.36 kB 574.36 kB = 100.83 kB 100.83 kB
oss-experimental/react-test-renderer/shallow.js +226.87% 0.07 kB 0.22 kB +94.12% 0.09 kB 0.17 kB
oss-stable-semver/react-test-renderer/shallow.js +226.87% 0.07 kB 0.22 kB +94.12% 0.09 kB 0.17 kB
oss-stable/react-test-renderer/shallow.js +226.87% 0.07 kB 0.22 kB +94.12% 0.09 kB 0.17 kB
test_utils/ReactAllWarnings.js Deleted 64.83 kB 0.00 kB Deleted 16.08 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
oss-experimental/react-test-renderer/shallow.js +226.87% 0.07 kB 0.22 kB +94.12% 0.09 kB 0.17 kB
oss-stable-semver/react-test-renderer/shallow.js +226.87% 0.07 kB 0.22 kB +94.12% 0.09 kB 0.17 kB
oss-stable/react-test-renderer/shallow.js +226.87% 0.07 kB 0.22 kB +94.12% 0.09 kB 0.17 kB
test_utils/ReactAllWarnings.js Deleted 64.83 kB 0.00 kB Deleted 16.08 kB 0.00 kB

Generated by 🚫 dangerJS against a4c5ff6

@eps1lon
Copy link
Collaborator

eps1lon commented Mar 6, 2024

It's a bit unfortunate that we never landed a deprecation warning. Instead of removing, can we throw in the entrypoint with a message pointing to the new package?

module.exports = require('react-shallow-renderer');
function ReactShallowRenderer() {
throw new Error(
'react-test-renderer is deprecated. To use react-test-renderer/shallow, import react-shallow-renderer directly. See https://react.dev/warnings/react-test-renderer'
Copy link
Member

Choose a reason for hiding this comment

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

Nit: "deprecated" usually doesn't mean "removed" but in this case, this is removed. I would say something like this instead:

react-test-renderer/shallow has been removed. See https://react.dev/warnings/react-test-renderer.

I'm not sure we should direct to react-shallow-renderer in the error message, or if we should mention it on the warning page instead. Really, people shouldn't be using shallow rendering at all, and direct people to that package would lead them in the wrong direction.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can move more detail to the warning page. My thinking for letting people know about the package movement is just to minimize the amount of blockers for adopting 19. If an app happens to use a bunch of shallow renderer in their test suite, hopefully they can drop in the original package we were reexporting and work on an incremental migration instead of being blocked completely.

@jackpope jackpope merged commit 1f9befe into facebook:main Mar 26, 2024
38 checks passed
@jackpope jackpope deleted the rtr-remove-shallow branch March 26, 2024 22:04
jackpope added a commit that referenced this pull request Mar 26, 2024
Based on
- #28497
- #28419

Reusing the disableLegacyMode flag, we set ReactTestRenderer to always
render with concurrent root where legacy APIs are no longer available.
If disableLegacyMode is false, we continue to allow the
unstable_isConcurrent option determine the root type.

Also checking a global `IS_REACT_NATIVE_TEST_ENVIRONMENT` so we can
maintain the existing behavior for RN until we remove legacy root
support there.
github-actions bot pushed a commit that referenced this pull request Mar 26, 2024
Based on
- #28497
- #28419

Reusing the disableLegacyMode flag, we set ReactTestRenderer to always
render with concurrent root where legacy APIs are no longer available.
If disableLegacyMode is false, we continue to allow the
unstable_isConcurrent option determine the root type.

Also checking a global `IS_REACT_NATIVE_TEST_ENVIRONMENT` so we can
maintain the existing behavior for RN until we remove legacy root
support there.

DiffTrain build for [bb66aa3](bb66aa3)
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
Based on
- facebook#28419

## Summary

The shallow renderer was extracted from the repo years ago and published
by enzyme: https://github.com/enzymejs/react-shallow-renderer

We no longer need to reexport under the react-test-renderer namespace.
People can import `react-shallow-renderer` as needed

## How did you test this change?

- Observe shallow.js in react-test-renderer package from standard build
- Run build with changes on this branch
- Observe no more shallow.js export in build output
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
Based on
- facebook#28497
- facebook#28419

Reusing the disableLegacyMode flag, we set ReactTestRenderer to always
render with concurrent root where legacy APIs are no longer available.
If disableLegacyMode is false, we continue to allow the
unstable_isConcurrent option determine the root type.

Also checking a global `IS_REACT_NATIVE_TEST_ENVIRONMENT` so we can
maintain the existing behavior for RN until we remove legacy root
support there.
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 React 19
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants