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

[RTR] Enable warning flag #28419

Merged
merged 1 commit into from
Mar 26, 2024
Merged

[RTR] Enable warning flag #28419

merged 1 commit into from
Mar 26, 2024

Conversation

jackpope
Copy link
Contributor

Summary

Based on

This PR

  • Silence warning in React tests
  • Turn on flag

We want to finish cleaning up internal RTR usage, but let's prioritize the deprecation process. We do this by silencing the internal warning for now.

How did you test this change?

yarn build
yarn test ReactHooksInspectionIntegration -b

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

react-sizebot commented Feb 22, 2024

Comparing: dbfbfb3...6b5e14c

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.92 kB 176.92 kB = 54.94 kB 54.94 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 173.32 kB 173.32 kB = 54.06 kB 54.06 kB
facebook-www/ReactDOM-prod.classic.js = 595.14 kB 595.14 kB = 104.51 kB 104.51 kB
facebook-www/ReactDOM-prod.modern.js = 578.40 kB 578.40 kB = 101.53 kB 101.53 kB
test_utils/ReactAllWarnings.js Deleted 65.55 kB 0.00 kB Deleted 16.18 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 65.55 kB 0.00 kB Deleted 16.18 kB 0.00 kB

Generated by 🚫 dangerJS against 6b5e14c

@@ -187,8 +187,7 @@ export const enableInfiniteRenderLoopDetection = true;
// during element creation.
export const enableRefAsProp = __NEXT_MAJOR__;

// Not ready to break experimental yet.
// Needs more internal cleanup
// Set to next major in ReactFeatureFlags.test-renderer
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we just flip it now to keep the main ReactFeatureFlags in sync with the test-renderer fork? It hasn't any practical effect at the moment but they should generally be in sync:

// TODO: This must be in sync with the main ReactFeatureFlags file because
// the Test Renderer's value must be the same as the one used by the
// react package.
//
// We really need to get rid of this whole module. Any test renderer specific
// flags should be handled by the Fiber config.

@eps1lon
Copy link
Collaborator

eps1lon commented Mar 23, 2024

Doesn't this also issue this warning when you'd use @testing-library/react-native? Are they expected to catch this warning? Otherwise the warning is noise for @testing-library/react-native users since they're not using a deprecated API directly. You're not supposed to move off of @testing-library/react-native but rather from bare react-test-renderer usage to @testing-library/react-native, no?

@rickhanlonii
Copy link
Member

@eps1lon yeah good point, @jackpope and I talked about that, we're going to address that before merging this so @testing-library/react-native doesn't get the warning message, either with a global like act sets or by publishing a separate react-native-test-renderer package.

@jackpope jackpope force-pushed the rtr-warning-2 branch 2 times, most recently from bf84bba to 04bb207 Compare March 25, 2024 14:29
@jackpope jackpope merged commit f73d11f into facebook:main Mar 26, 2024
38 checks passed
@jackpope jackpope deleted the rtr-warning-2 branch March 26, 2024 21:44
jackpope added a commit that referenced this pull request Mar 26, 2024
Based on
- #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
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
## Summary

Based on
- facebook#27903

This PR
- Silence warning in React tests
- Turn on flag

We want to finish cleaning up internal RTR usage, but let's prioritize
the deprecation process. We do this by silencing the internal warning
for now.

## How did you test this change?

`yarn build`
`yarn test ReactHooksInspectionIntegration -b`
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.
hoxyq added a commit that referenced this pull request Apr 15, 2024
Full list of changes:
* Look for a ReactMemoCacheSentinel on state
([gsathya](https://github.com/gsathya) in
[#28831](#28831))
* Use use() in the Cache if available
([sebmarkbage](https://github.com/sebmarkbage) in
[#28793](#28793))
* feat[devtools-fusebox]: support theme option
([hoxyq](https://github.com/hoxyq) in
[#28832](#28832))
* feat[devtools]: add package for fusebox integration
([hoxyq](https://github.com/hoxyq) in
[#28553](#28553))
* feat[devtools]: add method for connecting backend with custom
messaging protocol ([hoxyq](https://github.com/hoxyq) in
[#28552](#28552))
* Rename SECRET INTERNALS to
`__CLIENT_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE`
([sebmarkbage](https://github.com/sebmarkbage) in
[#28789](#28789))
* Flatten ReactSharedInternals
([sebmarkbage](https://github.com/sebmarkbage) in
[#28783](#28783))
* feat[devtools]: ship source maps for content scripts and ignore list
installHook script ([hoxyq](https://github.com/hoxyq) in
[#28730](#28730))
* Track Owner for Server Components in DEV
([sebmarkbage](https://github.com/sebmarkbage) in
[#28753](#28753))
* Move ReactDOMLegacy implementation into RootFB
([sebmarkbage](https://github.com/sebmarkbage) in
[#28656](#28656))
* Reland #28672: Remove IndeterminateComponent
([gnoff](https://github.com/gnoff) in
[#28681](#28681))
* Remove reference to deleted <Cache> in un-linted file
([josephsavona](https://github.com/josephsavona) in
[#28715](#28715))
* [be] Remove unshipped experimental <Cache> element type
([josephsavona](https://github.com/josephsavona) in
[#28698](#28698))
* Revert "Remove module pattern function component support"
([rickhanlonii](https://github.com/rickhanlonii) in
[#28670](#28670))
* Remove module pattern function component support
([gnoff](https://github.com/gnoff) in
[#27742](#27742))
* [RTR] Enable warning flag ([jackpope](https://github.com/jackpope) in
[#28419](#28419))
* Update error messages ([rickhanlonii](https://github.com/rickhanlonii)
in [#28652](#28652))
* fix[devtools/ci]: split profiling cache test for different react
versions and toEqual checker ([hoxyq](https://github.com/hoxyq) in
[#28628](#28628))
* Guard against legacy context not being supported in DevTools fixture
([eps1lon](https://github.com/eps1lon) in
[#28596](#28596))
* Use `declare const` instead of `declare var`
([kassens](https://github.com/kassens) in
[#28599](#28599))
* Update isConcurrent RTR option usage
([jackpope](https://github.com/jackpope) in
[#28546](#28546))
* Disable legacy context ([kassens](https://github.com/kassens) in
[#27991](#27991))
* Remove invokeGuardedCallback and replay trick
([sebmarkbage](https://github.com/sebmarkbage) in
[#28515](#28515))
* Remove remaining usages of ReactTestUtils in tests unrelated to
`react-dom/test-util` ([eps1lon](https://github.com/eps1lon) in
[#28534](#28534))
* fix[devtools/e2e]: fixed source inspection in e2e tests
([hoxyq](https://github.com/hoxyq) in
[#28518](#28518))
* Devtools: Display actual pending state when inspecting `useTransition`
([eps1lon](https://github.com/eps1lon) in
[#28499](#28499))
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.

5 participants