No whitespace warning #7081
No whitespace warning #7081
Conversation
ghost
added
| @@ -48,6 +48,7 @@ var ReactDOMTextComponent = function(text) { | |||
| this._mountIndex = 0; | |||
| this._closingComment = null; | |||
| this._commentNodes = null; | |||
| this._isWhitespace = typeof text === 'string' && text.trim().length === 0; | |||
There was a problem hiding this comment.
text in this case is not ensured to be a string. I'm also not sure if adding a property here is the right move. Maybe the check should be done in validateDOMNesting.js?
There was a problem hiding this comment.
We only care about this in DEV right?
There was a problem hiding this comment.
That's right. validateDOMNesting is a noop in production mode and, at the moment, it's the only place that cares about _isWhitespace.
Thanks for pointing that out, so this line should be changed to:
if (__DEV__) {
this._isWhitespace = typeof text === 'string' && text.trim().length === 0;
}Or did you have something else in mind?
hkal
force-pushed
the
no-whitespace-warning
branch
from
fba590a to
28614a6
Compare
| @@ -48,6 +48,11 @@ var ReactDOMTextComponent = function(text) { | |||
| this._mountIndex = 0; | |||
| this._closingComment = null; | |||
| this._commentNodes = null; | |||
|
|
|||
| if (__DEV__) { | |||
There was a problem hiding this comment.
@syranide - We now check for dev mode before adding this property
* Identify if content of ReactDOMTextComponent is only whitespace * Fixed misc. lint errors
hkal
force-pushed
the
no-whitespace-warning
branch
from
28614a6 to
e35e2de
Compare
|
@gaearon We good on this or does it need more work? |
|
|
||
| if (__DEV__) { | ||
| // validateDOMNesting uses this: | ||
| this._isWhitespace = typeof text === 'string' && text.trim().length === 0; |
There was a problem hiding this comment.
Just curious, since this might be somewhat of a hot path. Have you benchmarked this against REGEX.test(text)? With REGEX being a constant and not inline.
|
@hkal can you resolve the merge conflicts? Thanks! |
|
I already ended up fixing this in a different way in #7515. Thanks for sending this though! |
Resolves #6995
Includes some misc. lint fixes. Will squash if accepted.
cc @spicyj @jimfb