-
Notifications
You must be signed in to change notification settings - Fork 46.9k
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 module pattern function component support #27742
Conversation
@@ -53,7 +52,6 @@ export function describeFiber( | |||
case SuspenseListComponent: | |||
return describeBuiltInComponentFrame('SuspenseList', owner); | |||
case FunctionComponent: | |||
case IndeterminateComponent: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should keep it here for compatibility with React versions < 19
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good call, thanks!
@@ -220,7 +220,7 @@ export function getInternalReactConstants(version: string): { | |||
HostSingleton: 27, // Same as above | |||
HostText: 6, | |||
IncompleteClassComponent: 17, | |||
IndeterminateComponent: 2, | |||
IndeterminateComponent: 2, // removed in 19.0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think better way is to add an extra gte(version, '19.0.0')
check at the top of these if / else
statements, which will return the same ReactTypeOfWork
, but with IndeterminateComponent: -1 // Removed
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I originally did this but Seb pointed out, and I think this makes sense, that we really only need to create a new config by version when we modify the number used for a certain kind of component. In 19 there simply will be no IndeterminateComponent's in the tree but we don't really need to enforce this via devtools so we can just mark this as removed in a comment and if we have some other motivation to clean this up in the future we can get rid of it then.
It'd be interesting as an exercise to potentially collapse the config to be only when the tag numbers changed, it might make the tool slightly smaller and we'd have fewer branches for this tag map
5eaa4c6
to
85e7de6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neat
@@ -1338,7 +1336,7 @@ let hasWarnedAboutUsingContextAsConsumer = false; | |||
|
|||
// This would typically be a function component but we still support module pattern |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
old comment
@@ -55,7 +55,6 @@ export const enableSuspenseCallback = false; | |||
export const disableLegacyContext = false; | |||
export const enableTrustedTypesIntegration = false; | |||
export const disableTextareaChildren = false; | |||
export const disableModulePatternComponents = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR needs to rebase on #27739 and should wait for that feature flag to be enabled.
ad0f5a6
to
2e5b24b
Compare
#27742 will remove this feature flag altogether, this just already removes the dynamic flag for the Meta React Native build ahead of time.
732f992
to
4f22516
Compare
@kassens @rickhanlonii I rebased on main. This PR removes the flag entirely. Are we blocked on landing this because of RN? |
…s at this point. Supporting this pattern required React to have a concept of an indeterminate component so that when a component first renders it can turn into either a ClassComponent or a FunctionComponent depending on what it returns. While this feature was deprecated and put behind a flag it is still in stable. This change remvoes the flag, removes the warnings, and removes the concept of IndeterminateComponent from the React codebase.
4f22516
to
081a4a9
Compare
@gnoff no we can remove |
The module pattern ``` function MyComponent() { return { render() { return this.state.foo } } } ``` has been deprecated for approximately 5 years now. This PR removes support for this pattern. It also simplifies a number of code paths in particular related to the concept of `IndeterminateComponent` types. DiffTrain build for [cc56bed](cc56bed)
This reverts commit cc56bed.
> This is a redo of facebook#27742, but only including the flag removal, excluding further simplifications. The module pattern ``` function MyComponent() { return { render() { return this.state.foo } } } ``` has been deprecated for approximately 5 years now. This PR removes support for this pattern.
> This is a redo of facebook#27742, but only including the flag removal, excluding further simplifications. The module pattern ``` function MyComponent() { return { render() { return this.state.foo } } } ``` has been deprecated for approximately 5 years now. This PR removes support for this pattern.
> This is a redo of facebook#27742, but only including the flag removal, excluding further simplifications. The module pattern ``` function MyComponent() { return { render() { return this.state.foo } } } ``` has been deprecated for approximately 5 years now. This PR removes support for this pattern.
Remove module pattern function component support (flag only) > This is a redo of #27742, but only including the flag removal, excluding further simplifications. The module pattern ``` function MyComponent() { return { render() { return this.state.foo } } } ``` has been deprecated for approximately 5 years now. This PR removes support for this pattern.
Remove module pattern function component support (flag only) > This is a redo of #27742, but only including the flag removal, excluding further simplifications. The module pattern ``` function MyComponent() { return { render() { return this.state.foo } } } ``` has been deprecated for approximately 5 years now. This PR removes support for this pattern. DiffTrain build for [a73c345](a73c345)
) facebook#27742 will remove this feature flag altogether, this just already removes the dynamic flag for the Meta React Native build ahead of time.
The module pattern ``` function MyComponent() { return { render() { return this.state.foo } } } ``` has been deprecated for approximately 5 years now. This PR removes support for this pattern. It also simplifies a number of code paths in particular related to the concept of `IndeterminateComponent` types.
…k#28671) Remove module pattern function component support (flag only) > This is a redo of facebook#27742, but only including the flag removal, excluding further simplifications. The module pattern ``` function MyComponent() { return { render() { return this.state.foo } } } ``` has been deprecated for approximately 5 years now. This PR removes support for this pattern.
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))
The module pattern ``` function MyComponent() { return { render() { return this.state.foo } } } ``` has been deprecated for approximately 5 years now. This PR removes support for this pattern. It also simplifies a number of code paths in particular related to the concept of `IndeterminateComponent` types. DiffTrain build for commit cc56bed.
Remove module pattern function component support (flag only) > This is a redo of #27742, but only including the flag removal, excluding further simplifications. The module pattern ``` function MyComponent() { return { render() { return this.state.foo } } } ``` has been deprecated for approximately 5 years now. This PR removes support for this pattern. DiffTrain build for commit a73c345.
The module pattern
has been deprecated for approximately 5 years now. This PR removes support for this pattern. It also simplifies a number of code paths in particular related to the concept of
IndeterminateComponent
types.