SwitchTransition children may be equal if both keys are null #526
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
Add test for null key
If this was called with a key of the Router's location.key and this is null then the children were not equal and the transition was repeatedly fading in and out. This could be seen by opening a new window and browsing to a site where the user had a valid token.
Fix broken build, {null} not null
taion
reviewed
Jul 25, 2019
There was a problem hiding this comment.
If they have literally no key, then I don't see how we can say one thing or another. In this kind of case, why couldn't you just take e.g. "@@unknown" or something as a fallback key and be more explicit in application space, as opposed to forcing the library into potentially-surprising behavior?
| @@ -8,8 +8,7 @@ function areChildrenDifferent(oldChildren, newChildren) { | |||
| if ( | |||
| React.isValidElement(oldChildren) && | |||
| React.isValidElement(newChildren) && | |||
| oldChildren.key != null && | |||
| oldChildren.key === newChildren.key | |||
| ((oldChildren.key == null && newChildren.key == null) || oldChildren.key === newChildren.key) | |||
There was a problem hiding this comment.
Why would we want to treat null and undefined the same here?
There was a problem hiding this comment.
@taion, OK. This is how I initially coded it but decided it would be better handled in the component. I'll close the fork therefore.
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.
If the SwitchTransition's children both have a null key then they were not equal. If the Router's location has no key then the children were being continually re-rendered with the animation applied.