Move Component Class Instantiation into ReactCompositeComponent #2918
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
cc @spicyj |
sebmarkbage
force-pushed
the
fixcontexts
branch
from
c073a76 to
5cdf8d1
Compare
|
cc @zpao |
sebmarkbage
force-pushed
the
fixcontexts
branch
from
5cdf8d1 to
3e6c208
Compare
sebmarkbage
changed the title
| // Extract the component class from the element. Converts string types | ||
| // to a composite class which may have propTypes. | ||
| // TODO: Validating a string's propTypes is not decoupled from the | ||
| // rendering target which is problematic. |
There was a problem hiding this comment.
How so? Isn't componentClass.propTypes dependent on what ReactNativeComponent has injected?
There was a problem hiding this comment.
Yes, but it's problematic if there are multiple output environments. E.g. if an ART subtree was defined in terms of string based target nodes. Then we don't know which of the multiple environments this particular string will end up at.
Another case is HTML/SVG where the <title /> element means different things depending on which parent it ends up in. So they should have different propTypes depending on the parent. We don't really have a good way to solve this issue.
We need to move instantiation into the mount phase for context purposes. To do this I moved the shallow rendering stuff into ReactTestUtils and reused more of the existing code for it by instantiating a noop child. Everywhere we refer to the "type" we should pass it to ReactNativeComponent to resolve any string value into the underlying composite.
This should reenable reading this.context from getInitialState. Added a bunch of tests for this too.
sebmarkbage
force-pushed
the
fixcontexts
branch
from
3e6c208 to
9abd113
Compare
|
lgtm |
sebmarkbage
added a commit
that referenced
this pull request
Jan 23, 2015
Move Component Class Instantiation into ReactCompositeComponent
josephsavona
pushed a commit
that referenced
this pull request
May 15, 2024
This uses the compiler runtime from `react/compiler-runtime` by default unless `compilerRuntime` is specifified in the Babel options which then imports the runtime from there. The `useMemoCache` hook is now named `c` in accordance with 4508873 Unfortunately, I couldn't figure out how to import `react@beta` which already has that import as various react verstions were conflicting. If someone can figure this out it'd be fantastic. As a result, I had to update the default for the test runner to default the `compilerRuntime` option to `react` to preserve the previous behavior to import from `react`. Once upgraded to React 19, we should be able to remove that override.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We need to move instantiation into the mount phase for context purposes.
To do this I moved the shallow rendering stuff into ReactTestUtils and
reused more of the existing code for it by instantiating a noop child.
Everywhere we refer to the "type" we should pass it to ReactNativeComponent
to resolve any string value into the underlying composite.
After that I added context to the public instance constructor so that we can set it up before getInitialState.
Fixes #2898