Preact is checking a radio button without "checked" attribute on DOM

philipmw commented 4 years ago

Reproduction

or this standalone HTML file:

<!DOCTYPE html>
<html>
<head>
<script type="module">
import { h, Component, render } from 'https://unpkg.com/preact?module';

const radio1 = h('input', { type: "radio", name: "source", value: "1" });
const radio2 = h('input', { type: "radio", name: "source", value: "2", custom: "custom" });
const radio3 = h('input', { type: "radio", name: "source", value: "3", checked: "checked" });
const app = h('div', null, [radio1, radio2, radio3]);

render(app, document.body);
</script>
</head>
</html>

Steps to reproduce

Open the CodePen, or load the HTML above into your browser.

Expected Behavior

The third radio button is checked, the DOM has the checked attribute on the third input element, and my Enzyme/Cheerio unit tests can detect that the third radio button is checked.

Actual Behavior

The third radio button is checked ... but the DOM does not reflect this in any way that I can see. There is no checked attribute on any of the input elements, and my Enzyme/Cheerio unit tests are unable to detect any of the buttons as checked.

I don't understand how Preact is even checking the radio button, if not through the checked attribute.

In contrast, React 16 does apply a checked="" attribute.

marvinhagemeister commented 4 years ago

Ah the fun of attributes vs properties. Inspecting the element reveals that it is indeed checked correctly:

console.log(input.checked); // Logs: true

But for some reason browser don't reflect that back to an attribute.

philipmw commented 4 years ago

Yes... I am beginning to see that my expectation of having a checked attribute is wrong. jQuery prop() documentation explains:

Nevertheless, the most important concept to remember about the checked attribute is that it does not correspond to the checked property. The attribute actually corresponds to the defaultChecked property and should be used only to set the initial value of the checkbox. The checked attribute value does not change with the state of the checkbox, while the checked property does.

So it's likely that Preact is correct, even though it's somewhat different behavior from React's.

The actual problem I am having is that my unit tests are unable to verify that the radio button is checked. Maybe my problem lies with the enzyme-adapter-preact-pure project, in that the elements it's rendering in unit tests do not have this property. What do you think?

marvinhagemeister commented 4 years ago

Looping in @robertknight who maintains our enzyme adapter.

philipmw commented 4 years ago

Thank you! I'm glad to have one place to try to solve this issue.

In my tests, I tried this:

wrapper.find("input.dice").render().checked //undefined

and

wrapper.find("input.dice").render().prop("checked") //false

philipmw commented 4 years ago

I did some more research, and I believe I understand the root cause.

In Enzyme, I call .render() to get a cheerio object. This lets me inspect this component's attributes and properties, in jQuery lingo. According to jQuery, the checkedness of a radio button or checkbox is a property, not an attribute. Preact agrees, by not setting the "checked" attribute, only the property. (I have not found yet where that property is set in Preact.)

However, the way Enzyme hands the element to Cheerio is through the rendered HTML. If my assessment is right, this means that there is no way to read any property in Enzyme tests---only attributes.

React, in contrast, does set a checked attribute (in addition to whatever it does to properties), which allowed my Enzyme test to assert checkedness.

robertknight commented 4 years ago

However, the way Enzyme hands the element to Cheerio is through the rendered HTML. If my assessment is right, this means that there is no way to read any property in Enzyme tests---only attributes.

That's definitely not the case as a general statement. At work I have a large number of Preact + Enzyme tests that rely on looking at both properties and attributes.

Typically if you wanted to check the value of a prop passed to an HTML checkbox element created with JSX you would do something like:

wrapper.find("input.somecheckbox").prop("checked")

Given the above the value of the checked prop passed in the <input checked={...}/> expression will be returned and it doesn't matter whether Preact sets it as an attribute or a property.

What was the reason for using render() in the examples above?

If for a particular test you really wanted to look at the rendered DOM node then you would use mount(...) rather than shallow(...) and do something like wrapper.find("input.somecheckbox").getDOMNode().getAttribute("checked").

In general I recommend always using mount rendering. Mocking child components is a good practice if you are writing unit tests rather than integration tests but React/Enzyme's shallow rendering is not the best way to do this in my view. The problem is that in React shallow rendering works completely differently from normal rendering and this manifests itself in various subtle and not-so-subtle ways. I have a blog post here where I discuss this in more detail and propose alternative ways to achieve this: https://robertknight.me.uk/posts/shallow-rendering-revisited/.

philipmw commented 4 years ago

I see. Thank you, @robertknight. I am new to Enzyme and did not realize that I can check props without .render beforehand. Indeed that works!

I will read your blog post. Thanks for linking me to that.

I move to close this issue. It's been very educational. I had a number of misconceptions, but now that those are cleared up, I see that everything is working as designed.

preactjs / preact