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

SSR boolean attributes #1109

Closed
jamesbirtles opened this issue Jan 15, 2018 · 7 comments
Closed

SSR boolean attributes #1109

jamesbirtles opened this issue Jan 15, 2018 · 7 comments
Labels

Comments

@jamesbirtles
Copy link
Contributor

Svelte doesn't render boolean attributes properly on the server side.

For instance, for a button

<button :disabled>Hello</button>

This will render (when disabled = false)

<button disabled="false"></button>

which means the button actually is disabled

Here's a gist with an example and the ssr code it generates.

@Conduitry Conduitry added the bug label Jan 15, 2018
@Conduitry
Copy link
Member

The relevant code is here ... I'm not sure whether we want to simply make this so that it doesn't render the attribute at all when its value === false. I guess it already seems strange to me that the only time we're putting the bare attribute name into the output is when its value === true.

Would it be better to look up whether this is a boolean attribute and include or don't include it depending on its value's truthiness - and to always serialize the value of attributes that aren't boolean according to the spec? I had thought there was already a lookup somewhere in Svelte's source for which attributes were boolean, but either there's not or I can't find it.

@Conduitry
Copy link
Member

This is a bit messier than I thought. I didn't realize there were so many different boolean attributes. See here and do a text search for 'boolean attribute'. There are a couple things I don't know whether we need to worry about:

  • Should boolean attributes only be treated as such when they appear on certain elements?
  • Do boolean attributes that don't have a corresponding property exist? (Not strictly related to SSR, as all we can do there is deal with attributes - but does this impact the generated DOM code at all?)

@Rich-Harris
Copy link
Member

Finding a canonical list of boolean attributes is weirdly difficult. Might have to write a mini-parser that extracts them from https://www.w3.org/TR/html52/single-page.html#sec-boolean-attributes. I did find this list by @ArjanSchouten, though it's not clear where it's sourced from — Arjan, if you see this, can you help?!

My suspicion is that it's okay to treat a particular attribute that we know to be boolean (such as disabled) as boolean in all cases, since outside elements on which the attribute is valid (e.g. <button>) it presumably has no meaning anyway. But if we wanted to cover all the bases then the IDL on the aforementioned page should give us an exhaustive elementName: booleanAttributes map.

@Conduitry
Copy link
Member

Conduitry commented Jan 20, 2018

If we're fine with treating attributes as static regardless of the element they're on, that sounds like it's probably a good enough solution for now. That would really only affect SSR stuff. (For DOM code, we'd continue to use the attributes-to-props lookup as it exists now, and rely on browsers to do the right thing.)

Using the list from that gist (and rendering attributes in SSR according to whether they're in that list, not by whether their value === true) seems like a good path because it's easy to implement and will probably cover almost all cases. If someone runs into an issue with that, it seems plausible that they'll be a bit more knowledgeable about boolean attributes and can help guide us 😀

Edit: Hm, looks like we'd also need to output the attribute as a boolean attribute when its value is === true, even without it being in the known boolean attribute list.

@Rich-Harris
Copy link
Member

I'm good with that approach 👍

Rich-Harris added a commit that referenced this issue Jan 20, 2018
fix handling of boolean attributes in SSR (#1109)
@ArjanSchouten
Copy link

@Rich-Harris I created this list by going through w3schools.com's HTML tag list manually. I needed it for a side project from some time ago: https://github.com/ArjanSchouten/HtmlMinifier. In the meanwhile the list can be outdated.

@Rich-Harris
Copy link
Member

@ArjanSchouten thank you. The list is extremely helpful, and I'd wager it's still mostly correct.

Released 1.54 with @Conduitry's fix — thanks @UnwrittenFun

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

No branches or pull requests

4 participants