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

Attribute Ordering #43

Closed
rturnq opened this issue Jul 3, 2019 · 5 comments
Closed

Attribute Ordering #43

rturnq opened this issue Jul 3, 2019 · 5 comments
Labels
question Further information is requested

Comments

@rturnq
Copy link
Member

rturnq commented Jul 3, 2019

I've been playing around with Solid and came across an issue I found interesting. There is at least one case where an HTML element has an attribute with a dependency on another one of its attributes, which means that the order of their application becomes important.

I've documented some examples here https://codesandbox.io/s/attribute-order-example-pmdqh

The range input <input type="range" /> has a min and max attribute which are taken into account when setting the value. This makes sense, you have to clamp the value set in order to keep it in range.

Essentially, if you apply the value before min and max, you may (depending on the numbers used) get a different outcome than applying the min and max first. This is easy enough for an author to fix by simply changing the ordering in their JSX; however, this brings up a few questions

  • Does the DOM Expressions library guarantee ordering of application at runtime based on the ordering found is the JSX?
  • Is this something the user should have to be aware of? I found the issue trying to create the same simple app in React, Vue and Solid - the others appear to prevent the issue.
  • If a user's code sets some state that updates all of these attributes at the same time (ie setState({ min, max, value });) could this cause a problem? What determines the order they are applied?

I'm not sure what the answers are but it seems like the best solution would be to uncover all of these attribute dependencies (maybe other elements have them too) and take them into account when applying updates, possibly rearranging the calls so the application author and end user have a consistent outcome.

I really like this project and I'm blown away that it seems to have everything I love about React (JSX, functional components, hooks) in such a tiny and speedy library. Good work!

@ryansolid ryansolid added the question Further information is requested label Jul 3, 2019
@ryansolid
Copy link
Member

ryansolid commented Jul 3, 2019

Thanks, for taking the time to go into this in detail. Most of what you witnessed makes sense to me. I didn't realize the DOM API's were so order dependent. It makes sense. It's just interesting that you could stumble on to that just using pure JS.

Basically the any given template the code generation looks like this:

  1. Clone template with all it's static attributes
  2. In JSX order for expressions and then children: create computations and set all values
  3. Return root element

From there the elements are attached and on value updates computations re-fire to update element values. Save from some nested chain dependencies (unlikely in this case, like value setting the max) updates should run again in JSX order. (EDIT actually I need to double check this).

React and Vue will not have this issue since they use Virtual DOM which will apply all updates in JSX order I imagine. The technique I use to initially render is a variation of what Google's lit-html and most Tagged Template Literal libraries(HyperHTML, lighterhtml, Haunted) use so I imagine they have similar issues. I will try to see how they look at this. But I'm yet to find any issues in their repos around this.

As you noted I should document that static attributes are all applied first in HTML parsing order.

@ryansolid
Copy link
Member

Yeah it looks like lit-html behaves the same way. I guess this is a problem inherent to this template cloning mechanism where static and dynamic parts are separated. The same mechanism WHATWG is trying to standardize for templating with Web Components. That's awkward. I wonder why I haven't seen this reported anywhere before for these libraries.

@rturnq
Copy link
Member Author

rturnq commented Jul 3, 2019

I didn't have time earlier to see how others handled it but it looks like it came up and was fixed in React: facebook/react#7486. Vue has an open issue for it vuejs/vue#9515 and it sounds like they are suggesting users just ensure the order themselves. Vue's v-model binding seems to get around the issue - it must run after the other attribute bindings.

I would now expect any UI library that sets DOM attributes would have encountered this issue or is vulnerable to it. Even with a virtual DOM setting the attributes is not atomic so somehow you have to decide which one to set first. Looks like React chose to ensure certain attributes are applied in a specific order. They also make sure to set type first since it looks like there may be some quirkiness there too.

I'm not sure which direction you want to go, it appears there is precedent to handle it in the library or simply document it. I personally think it makes sense for the lib to handle it as writing JSX should feel like writing HTML where it doesn't matter in what order you specify the attributes on a tag.

@ryansolid
Copy link
Member

Yeah the challenge with template cloning is that you cannot even ensure the JSX(or Tagged Template Literal) order if you have a combination of static and non-static attributes. I'd have to not apply attributes statically at all which gets rid of a lot of the advantage. Otherwise as you said I could re-order based on name in the compiler. It feels a little loose in terms of consistency piece but I also get it's the type of issue no one would really realize.

Looking at the Vue issue, this apparently is a Chrome thing? I wonder if it's spec. Mostly in that as this template cloning approach I use is being actively worked on by people involved in setting the spec. And with this template cloning method it is impossible for me to retain template order for non-static parts. So unlike Vue or React there is no ability to even re-order the fields wholistically if I wanted to without looking into destroying benefits of template cloing. I think that is much too messy for the library to solve. Since Solid is so aligned with the Browser I hope that the Browser vendors figure out a way to address this. I'm looking to Google's lit-html or perhaps this specification to address this.

For now I agree this should be documented.

@rturnq
Copy link
Member Author

rturnq commented Jul 3, 2019

Yeah, I guess the only thing you could do is rearrange the order of dynamic attributes but like you said, it could still lead to an inconsistent outcome for the user so documentation would still be required.

I just tried that example I linked in Edge and Firefox, both exhibit the same issue as Chrome. I didn't really see anything explicit in the HTML Standard but MDN suggests this is expected behavior

If an attempt is made to set the value lower than the minimum, it is set to the minimum. Similarly, an attempt to set the value higher than the maximum results in it being set to the maximum.

Anyways, documentation seems like the answer here. Feel free to close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants