Cannot set property to false
DxCx opened this issue · 17 comments
Hey,
I have html ast which i am parsing, with some of the properties being "false" (bool false not string).
they are boolean properties and i expect them to be either true or false.
After a processing of hast-to-hyperscript, those props are completely gone.
browsing the code abit, ive found those lines:
Lines 147 to 156 in 5dd6b51
(specificlly line 151 (value === false))
which i can understand why would you filter null and undefined, i don't understand why false is being filtered.
looking at the hast github again, i dont see any reasoning for that..
can you please explain this behavior? or maybe that is just a bug?
Because the HAST value false, as in, hidden: false, does not output the attribute to HTML and the like.
If you want the word false to be present in the HTML and the like, you can use a string, such as hidden: 'false'!
For the spec, it’s described here, in property values.
Why do you think this shouldn’t be the case? How are you creating the AST?
I am using this library as part of markdown <-> react engine
In my case i have property="true/false"
Then a property mapper is processing it to boolean true/false and the hast is saved.
Then on client side i use rehype-react (uses this library)
So from what i get is that booleans has to be by definition default to false and only if mentioned they should be treated as false :/
Well, what the property mapper? I believe it's a big there 🤷♂️
bug* gah sorry!
I have same issue. My usecase is to generate realtime preview for markdown editor.
For the spec, it’s described here, in property values.
I read property values, but I'm not sure why the value of false is to be ignored.
It says "non-standard values are retained to allow plug-ins and utilities to inspect them" and "null and undefined must be ignored".
I think that falsy values (except null and undefined) should be retained.
In React, properties must be always passing or non-passing. Flipping(passing or not) causes a warning as below:
Warning: A component is changing an uncontrolled input of type checkbox to be controlled. Input elements should not switch from uncontrolled to controlled (or vice versa). Decide between using a controlled or uncontrolled input element for the lifetime of the component. More info: https://fb.me/react-controlled-components
working sample is here: https://berlysia.github.io/example-for-hast-to-hyperscript-14/
i was changing my implemenation abit so it will act as menetioned here.
if property does not exists it defaults to false
if exists then value (if set) is ignored, and it's set to true.
(hence, both isHidden and isHidden="true", works)
@berlysia Is the problem maybe that checked should be passed as defaultChecked in React?
Is the problem maybe that checked should be passed as defaultChecked in React?
In short: No.
defaultChecked in React is wrapped, not passed directly to input element. Updating checked will re-render the component, but updates to defaultChecked must be ignored.
As documented in Uncontrolled Components - Default values,
In the React rendering lifecycle, the value attribute on form elements will override the value in the DOM. With an uncontrolled component, you often want React to specify the initial value, but leave subsequent updates uncontrolled. To handle this case, you can specify a defaultValue attribute instead of value.
Also I verified to use defaultChecked instead of checked. It doesn't work in Chrome and Firefox, but works in Safari and Edge(maybe it's a bug of React related to facebook/react#10150).
It's suitable to use checked property.
@DxCx Ahh okay, thanks for clarifying!
As HAST can represent all of HTML, and all properties on inputs (and other elements) could change, implementing just checked is probably only a tiny part of the solution. What other properties could result in this error? Everything? 🤔
checked and value on input, select and textarea elements cause the warning mentioned above.
However, because of property values, this issue can apply to all properties on all elements in HTML. HAST can represents superset of HTML.
Filtering inappropriate values would have better to be the responsibility of the consumer side (as your hast-util-to-html) or specialized filter library (as your hast-util-sanitize), not in a part of hast-to-hyperscript. We have no chance to handle non-standard values now, contrary to HAST's mindset.
Alright, I’m convinced! Could we try and ignore these two lines? Maybe it works for all other hyperscripts, but if not, we can scope this to just react?
For clarification, could you tell me what is "these two lines"?
Well, yeah thats what i believe, at least null..
But right now its more like html behavior
I have a similiar issue. In my case it is not a false value, but an array.
My node has properties like:
{
params: ["param1", "param2", "param3"]
}These lines in hast-to-hyperscript serializes the array into a string
if (value !== null && typeof value === 'object' && 'length' in value) {
// Accept `array`. Most props are space-separater.
value = (info.commaSeparated ? commas : spaces).stringify(value)
}As was the case with the false values, when dealing with react elements, I would expect the properties to apply to the react element as-is. So that the react elements receive the actual properties that are present in the HAST.
Would it be possible to not transform the properties in case of react elements?
Could you open a new issue? Otherwise it’s very easy to loose this comment.