fix(engine-core): dynamic class with empty string results in hydration mismatch #4684
Conversation
| @@ -664,7 +664,7 @@ function validateClassAttr( | |||
|
|
|||
| const elmClassName = getAttribute(elm, 'class'); | |||
|
|
|||
| if (!isUndefined(className) && String(className) !== elmClassName) { | |||
| if (!isUndefined(className) && String(className) !== elmClassName && className !== '') { | |||
There was a problem hiding this comment.
This isn't totally right. If the client class name is '' but the server class name is 'foo' then there will be no mismatch when there should be.
There was a problem hiding this comment.
Thanks! I figured some case wasn't covered here.
|
Thanks @nolanlawson . Should I try fixing this once you merge the new tests? Or are you taking care of it as well? |
|
@cardoso Feel free to go ahead! If you don't get to it, I'll get to it eventually. 🙂 |
| if (!isUndefined(className) && String(className) !== elmClassName) { | ||
| if ( | ||
| !isUndefined(className) && | ||
| String(className) !== elmClassName && | ||
| // No mismatch if SSR className is empty and the client-side class is null | ||
| !(className === '' && isNull(elmClassName)) | ||
| ) { |
There was a problem hiding this comment.
This if is starting to get hairy, but the auto-formatting gave me the idea of adding this comment. I think this is OK.
There was a problem hiding this comment.
Could also be (className !== '' || !isNull(elmClassName)) but I found this way more readable and easy to explain via the comment.
|
/nucleus test |
|
/nucleus test |
Details
Hopefully this is right, but if not there should be some test that fails.
Closes #4661
Does this pull request introduce a breaking change?
Does this pull request introduce an observable change?
No hydration mismatch on dynamic class with empty string 😄
GUS work item