Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Boolean DOM properties coerce empty string to false, contrary to HTML standard #13400

Open
motiz88 opened this issue Aug 14, 2018 · 3 comments
Open

Comments

@motiz88
Copy link
Contributor

motiz88 commented Aug 14, 2018

This is in kind of the same space as #13372 and is an offshoot of my attempt to better model React DOM props in Flow.

tl;dr: Should React warn when the value "" is passed into a known boolean DOM prop?


Do you want to request a feature or report a bug?

Depends on interpretation 😅 This is possibly a bug, definitely an inconsistency worth mitigating IMHO.

What is the current behavior?

React normalises values supplied to known DOM boolean props (e.g. readOnly) such that passing the empty string "" (being falsy in JavaScript) results in the corresponding attribute being omitted from the HTML output. However, in HTML, the empty string is a truthy value in this context; it's one of the values that the standard specifically allows in boolean attributes.

The above is a potential source of confusion in itself, but React 16's handling of unknown attributes gives rise to the following hypothetical scenario: a new DOM boolean attribute foobar is introduced, some people write JSX code that uses it as foobar="" (passed through to HTML, truthy), and later React adds foobar to its internal whitelist in a minor/patch version and starts processing it as a boolean (JS falsy, omitted from HTML); this would technically be a breaking change for those people.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem.

https://codesandbox.io/s/y0pmz9149x

What is the expected behavior?

There is definitely a clash of expectations here at the interface of JS and HTML.

  1. Coming from JS, "" is falsy and treating it as such in a "boolean" prop is fine; from this perspective, the current behaviour is justifiable.
  2. Coming from HTML, it might not be obvious that React is doing this "extra" processing and deviating from what's clearly stated in the HTML spec; from this perspective, the current behaviour is surprising.

There probably isn't justification for changing React's actual handling of "" (not least for fear of breaking code that relies on this long-standing behaviour, see version information below), but perhaps a warning about the ambiguity is warranted, a la #13372?

Note that a warning won't fully mitigate the worst-case scenario I mentioned above (since we can't warn about a prop that we don't know is a DOM boolean), but at least it would give some signal after the React version update that the code might not be doing the expected thing anymore.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

Versions of React as far back as 0.14 (and probably way older) process whitelisted boolean DOM props the same way.

@motiz88
Copy link
Contributor Author

motiz88 commented Aug 14, 2018

Another fun inconsistency I've just noticed is that BOOLEANISH_STRING props (e.g. draggable, spellCheck) treat "" as truthy (the way HTML does), the opposite of how BOOLEAN props behave. This too has been the case since React 0.14 or earlier.

@gaearon
Copy link
Collaborator

gaearon commented Aug 14, 2018

Maybe we should always warn when passing a string to a boolean attribute? Seems like it's always counter-intuitive what you'd get.

@motiz88
Copy link
Contributor Author

motiz88 commented Aug 15, 2018

@gaearon Yeah, I'd definitely be on board with that as a way to unify this and #13372.

Although, thinking about the case of a newly-specced HTML attribute: Looks like currently foobar="foobar" is the most reliable thing to do, as it will behave the same whether or not React DOM has foobar in its whitelist. So maybe that's one tentative argument against a blanket warning on string values passed to boolean attributes, and for focusing on actually-ambiguous cases instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants