Fix problems with empty component ID registration #2503
Merged
+253
−163
Conversation
This file contains hidden or 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
| */ | ||
| function getEmptyComponent() { | ||
| var ReactEmptyComponentType = function() {}; | ||
| ReactEmptyComponentType.prototype.render = function() { |
There was a problem hiding this comment.
(My first bare class! Am I doing it right?)
There was a problem hiding this comment.
If you uses classes it actually transforms to the same thing :)
class ReactEmptyComponentType {
render() {
...
}
}There was a problem hiding this comment.
Hah, I meant my first bare React component class. I can use class if that's preferred.
|
We successfully managed to treat "string" and "number" as ReactNodes (where we previously only allowed ReactElements). Perhaps we can do the same for "null"? |
Fixes facebook#2493, also closes facebook#2353 as it incorporates a variant of that fix. - Instead of having getEmptyComponent return `<noscript />` directly, wrap it in a class that we can easily identify later as being an empty component - Cache the empty component element instead of recreating one each time - Avoid touching the nullComponentIdsRegistry dictionary at all when not dealing with empty components - Move empty-component tests to a separate file Test Plan: jest
|
Probably. Going to land this as is then work on doing that separately. |
# for free
to join this conversation on GitHub.
Already have an account?
# 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.
Fixes #2493, also closes #2353 as it incorporates a variant of that fix.
<noscript />directly, wrap it in a class that we can easily identify later as being an empty componentTest Plan: jest