input readonly list-attribute

[marcusasplund]

Fails in rendering an input with attribute 'list' user for targeting datalist-element
Removing the attribute renders both input and datalist elements, then unconnected and useless of course.

Console message:
Cannot assign to read only property 'list' of object '#'
at setElementData...

Reproduced in glitch
https://glitch.com/edit/#!/wry-autocomplete

import { h, app } from 'hyperapp' // 0.7.1
// import {h, app} from '../hyperapp/index' // 0.8.0, unreleased


const dataList = () =>
  <datalist id='browsers'>
    <option value='Chrome'></option>
    <option value='Firefox'></option>
    <option value='Internet Explorer'></option>
    <option value='Opera'></option>
    <option value='Safari'></option>
    <option value='Microsoft Edge'></option>
  </datalist>

const view = () =>
  <div class='container'>
    <label>Choose a browser from this list:
    <input list='browsers' /></label>
    {dataList()}
  </div>            

app({view})

[jorgebucaran]

@marcusasplund Hi 👋!

I just woke up, so I haven't looked at this in good detail, but smells like another use case we fixed with keys #172?

@zaceno Do you have any comments?

[zaceno]

@jbucaran - Looked at the glitch-example @marcusasplund provided. As far as I can tell, it's got not nothing to do with keys.

Not sure exactly what the problem is. The relevant lines of code are (in setElementData)

     element.setAttribute(name, value)
     if (element.namespaceURI !== "http://www.w3.org/2000/svg") {
         // SVG element's props are read only in strict mode.

        element[name] = value
     }

the line that throws the exception is element[name] = value. Not sure why. Possibly because the list attribute is newish HTML5. Maybe there's some special way browsers treat it differently to other attributes.

The thing I don't get, though, is: We already do element.setAttribute(name, value) higher up. What does element[name] = value give us?

Even though using keys won't fix the issue, the setElementData function has been changed on master since 0.7.1, so it'd be interesting to see how this behaves on master.

[marcusasplund]

@zaceno Yup. I tried to switch from 0.7.1 to src from master in the glitch provided. Same behaviour. But commenting out line 184 in src/hyperapp/app.js (in the glitch) all elements renders and works as expected.
// element[name] = value
Haven't thouroghly tested, but at least it does not produce any console errors.
For me it looks like a bug; trying to assign one more time in an unappropriate way.

[jorgebucaran]

@marcusasplund This is a bug then, perhaps we need to be careful about some attributes or just turn off "use strict", which I'm almost sure if you can turn it off the problem would go away.

[zaceno]

I ran the tests on master with the line element[name] = value commented out ( in fact I took out the whole block: https://github.com/hyperapp/hyperapp/blob/master/src/app.js#L180:L191 ). All passed.

@jorgebucaran, do you know off hand of any reason why we need the element[name] = value at all?

(Side note: the reason I took out the entire code block surrounding that line, is because it was dealing with selectionStart/End in text inputs. I think now that we have keys, that's all more of a 'nice to have' rather than a 'must have', since a user could just key their textinput and the problem would be solved. Sure it's a little bit of a hassle for the user, but if we need to get rid of bytes, it's an option :) )

[jorgebucaran]

Well for once we use element[name] = value to set event handlers.

All passed.

This tells me, that we're lacking on tests then!

[FlorianWendelborn]

@marcusasplund Is there any reason why you use dataList() instead of <DataList/>? That's the usual way to use JSX.

[marcusasplund]

@dodekeract Nope.
Pretty newbie to jsx, i tried to find out now where i first got this syntax, but i cant remember.
I'd better read up on jsx :-) That component-/xml-like syntax feels a lot better. Thx for the heads up.
EDIT: actually i found it in the hyperapp-todo-mvc
So, which one to prefer? A matter of taste?
https://glitch.com/edit/#!/hyperapp-todomvc

[jorgebucaran]

@marcusasplund I seriously need to rewrite the entire todomvc app. When I first wrote it, I used Hyperx, later I tried to rewrite it in JSX, but didn't do a great job at it. So now it's not really clear what it is.

Things will get better after 0.8.0! 🙏

[zaceno]

@jorgebucaran: Ah, that makes sense.

I have some thoughts for a PR to fix this, but I'm going to hold off until 0.8.0. There's enough dust in the air :)

Either way, I agree: clearly more tests are needed if I can comment out an entire code block with no tests failing 😛

[jorgebucaran]

@marcusasplund I think this is fixed in >=0.8.1. Can you give it a shot? 🙏

[marcusasplund]

Yup. I can confirm that it works as expected now. Good work!
Updated to 0.8.1 https://glitch.com/edit/#!/wry-autocomplete
(Heads up: Glitch is a bit quirky/slow for me atm)