React: Checked attribute not in DOM

Created on 16 Jun 2016 · 14Comments · Source: facebook/react

Do you want to request a _feature_ or report a _bug_?

AFAIK, this is a bug.

What is the current behavior?

When setting checked on a checkbox, the checkbox indeed is checked, but the attribute isn't actually in the DOM. This means that checkbox will be targeted by :checked, but not [checked]. In my case, it is a blocker regarding Selenium-powered tests as we use XPath to target elements and [@checked] therefore doesn't work either.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar (template: https://jsfiddle.net/reactjs/69z2wepo/).

Check this out: https://jsfiddle.net/cw4hjLav/1/

Quick copypasta from my browser console (I ran those right after loading the fiddle):

document.querySelector('input[type=checkbox]')
<input type="checkbox">
document.querySelector('input[type=checkbox]:checked')
<input type="checkbox">
document.querySelector('input[type=checkbox][checked]')
null

The behaviour is the same with defaultChecked.

What is the expected behavior?

The checked attribute actually is in the DOM whenever relevant.

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

Tried this out with React 15 (the fiddle uses 15.0.1, my app uses 15.1.0), with both Firefox 47.0 and Chromium 50 on Ubuntu 16.04.

I previously stumbled upon #3005 but didn't find the answer there, as my issue doesn't seem to be preventDefault()-related. Thanks a lot!

Source

neemzy

👍2

All 14 comments

cc @jimfb - is this the same as #6478 or was value handled differently than checked?

zpao on 16 Jun 2016

IIRC, value was handled differently than checked. I'd be more than happy to submit a PR to do the same for checked (should be trivially easy, value was much more complicated), if you'd like me to do so :P. Should I create a PR?

jimfb on 16 Jun 2016

I'm actually not sure this ever worked. It looks like we have to set checked via the property so we would just be duplicating work to set the property.

And actually just trying this with straight DOM, it looks like even browsers are only setting the property when clicked, so I'm not sure there's anything we should do here.

zpao on 16 Jun 2016

@zpao The reason a bunch of people are running into this is because in React 14 we set innerHTML. So the attribute was correct for initial render (which is generally the only thing people were testing). The attribute was not updated for updates, but most people didn't notice, because people rarely test updates.

But yes, this was never officially supported. Though it might be nice just for consistency / to avoid having people banging their heads against the wall trying to figure out why it isn't working (since the reason isn't very discoverable).

jimfb on 16 Jun 2016

I can confirm it must have worked with React 14 since the team wrote the tests back when we were using it. Supporting this (for initial render only, or for all of them) would be great IMO, but I'd perfectly understand it cannot be (easily) done, or isn't relevant overall. I think the docs should at least reflect this though (the Forms page maybe?) - I'd be happy to PR the doc change if it doesn't require an in-depth explanation of why it isn't supported.

Anyway, I'll try working around it by adding a data-checked attribute relying on the same prop and testing that instead in the meantime. Please let me know what you guys decide :) Thanks!

neemzy on 17 Jun 2016

@neemzy Why not just make a function that traverses the document and updates the attribute to reflect the property? (for your tests)

syranide on 17 Jun 2016

👎9 👍1

@syranide That's a path I can see going towards an obscure, forgotten implementation detail that people will rant about in the future. That's a path I'm not willing to follow.

~~Anyway, my tests suddenly turned green this morning for no reason. I'll keep you updated.~~ I had mistakenly rollbacked to React 0.14... :D

neemzy on 17 Jun 2016

👍1

Is there anything I can do to help regarding this? :)

neemzy on 21 Jun 2016

FYI I ran into this about a year ago #4646

jlas on 21 Jun 2016

@neemzy This is not my expertise so perhaps I'm speaking out of turn. But this seems like a problem with your testing environment that assumes things about HTML that are not guaranteed (using XPath to query for checked checkboxes). To put it differently, your method does not work for checkboxes that a user (and many libraries) has interacted with either, the checked attribute only reflects the initial value. So yes it's kind of shitty to patch the DOM as I mentioned above, but it's about working around a problem with your usage of XPath and not actually a bug in React. If this is part of a larger testing framework (Selenium) then this is perhaps something they should consider finding a work-around for.

On another note, what's the problem with just querying checkboxes and then applying a filter on it removing ones that are unchecked. That's how you would otherwise do it I assume?

syranide on 22 Jun 2016

@syranide Correct, not actually a correctness bug in React. But users do find it surprising and confusing, especially since what is shown in the browser devtools is the attributes. By the principle of least surprise, it might be worth fixing this on our end.

jimfb on 22 Jun 2016

👍1

@jimfb Sure, but then they'll be even more confused when they have a controlled component with simulated interaction (which has never worked), or when they have an uncontrolled component with simulated interaction (which should never update the attribute), or use a library that interacts with a checkbox. Also note that this should really extend to all other user-interactive properties then, such as value, muted, etc. If you're not solving everything then you're making it worse... not to be snide, but principle of least surprise right? :P In a sense, the current behavior is the best behavior, because it _never_ works, you will never get the impression that it does in React.

IMHO, as you know I always prefer strictness, bad behavior should be uncovered as soon as possible or you are effectively encouraging it, which means they'll soon be back asking why it doesn't work for X and Y. Fix the problem at the source. I don't really care, but IMHO doing this seems like a charitable thing but it's actually the opposite.

syranide on 22 Jun 2016

As Web is a living standard, Since JSX is to compiled to HTML, checked attribute should be there.

JSX to (agnostic to rendering) should support HTML regardless they want to become the standard themselves.

Or the JSX should step up(provide funding) to become the next HTML, or at least don't cause more issue that finally fixed compatibility (:checked CSS property) in all browser.

Face it, JSX is not going to be success, if the rendering dom layer is so bad, the good point of JSX is always just the JavaScript Expressions part, which got it correct

ernestchanjq on 4 Jul 2017

👍1

As Web is a living standard, Since JSX is to compiled to HTML

This is a misunderstanding. JSX is not compiled to HTML. It’s compiled to JS, and is only meant to represent the UI your components render to.

React still uses JS under the hood. If you used browser JS DOM APIs (that have nothing to do with React itself), you know they don’t automatically update attributes. Again, this is not just React’s decision, but it’s how browsers work.

So it’s not at all obvious that React should update attributes. We’ve tried doing this in some cases for a while due to popular requests, but it turned out to be a rabbit hole of edge cases and inconsistencies, and I’m not sure we’ll continue doing it in the future.

Please don’t rely on React changing attributes in the DOM. In the case above, :checked is a reasonable thing to search for, but [checked] is not.