sveltejs / svelte

web development for the rest of us
https://svelte.dev
MIT License
80.05k stars 4.25k forks source link

$$props and $$restProps boolean attributes #7346

Closed phiberber closed 2 years ago

phiberber commented 2 years ago

Describe the bug

When spreading and there's a custom-attribute in $$restProps or $$props, Svelte treats it as a Boolean Attribute, but it sets the value of the attribute to "true", which is against HTML Specs:

To be clear, the values "true" and "false" are not allowed on boolean attributes. To represent a false value, the attribute has to be omitted altogether. This restriction clears up some common misunderstandings: With checked="false" for example, the element's checked attribute would be interpreted as true because the attribute is present.

This is preventing me currently from using WindiCSS & UnoCSS Attributify mode, as <Group flex> results in <div flex="true"> instead of <div flex>.

Reproduction

https://svelte.dev/repl/4ea0a9acf5944fc3a8957f7244bf6ceb?version=3.46.4

Logs

No response

System Info

System:
    OS: Windows 10 10.0.22000
    CPU: (8) x64 Intel(R) Core(TM) i7-2600 CPU @ 3.40GHz
    Memory: 4.85 GB / 15.94 GB
  Binaries:
    Node: 16.13.2 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.15 - C:\Program Files\nodejs\yarn.CMD
    npm: 8.4.0 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Chrome: 98.0.4758.102
    Internet Explorer: 11.0.22000.120
  npmPackages:
    svelte: ^3.44.0 => 3.46.4

Severity

annoyance

Conduitry commented 2 years ago

Are you requesting that $$props/$$restProps change their behavior, or that spread attributes change their behavior?

I don't think $$props or $$restProps should change. At that point, these are props, not attributes, and we shouldn't be trying to match HTML spec behavior.

I might be convinced that spreading attributes with values of true or false should respectively set an attribute with an empty string or remove the attribute, but I'm concerned with that being a breaking change for people who need the current behavior.

In the meantime, you could manually filter the object before using it as a spread attribute.

phiberber commented 2 years ago

I think that you should care about HTML Specs at the point where you're outputting HTML, which is fundamentally what Svelte does. The same happens with TypeScript and JavaScript, where it outputs ECMAScript compatible Javascript code.

This may be a breaking change to some current projects, that's why I think it should be added to a major version of Svelte like Svelte v4, where a migration is most of the times expected.

The change I'm proposing isn't much about how $$restProps and $$props work, but more about how attributes and properties relate between themselves. If a property is equal to "true", its attribute should have an empty value or no value at all, following the spec.

Currently I'm not using any work-around, just started using classes for things I can't use the attributify mode.

bluwy commented 2 years ago

Is flex a boolean attribute? If <Group flex> results in <div flex="true">, that looks right to me. There's only a particular set of standard boolean attributes according to the spec, that Svelte hardcodes in its compiler.

And IIUC you're passing flex down to the component as a a way to style it using WindiCSS attributify mode? I'm not sure that works considering WindiCSS has to compile the attributes statically, unless I'm missing something. But nonetheless, I'm not a fan of attributify mode, it breaks HTML semantics 😬

phiberber commented 2 years ago

Closing the issue, custom attributes do not follow HTML specs, and that's the whole point.

ZerdoX-x commented 2 years ago

REPL

This is really annoying tho. I can't build UI library without dublicating all boolean props to script section using "export".

Even docs take into account this usecase: svelte.dev/docs#template-syntax-attributes-and-props

image