Lwc: Remove the attribute validation done by the template compiler

[pmdartus]

Today the template compiler validate all the attributes on standard HTML elements. The compilation fails if an HTML element is associated with an unknown attribute.

I would like to remove this restriction for multiple reasons:

• it's a lot of maintenance on the compiler.
• there is no up-to-date list of all the available attributes on all the standard HTML elements. And because of this, we are aware of the missing attributes when a developer runs into a compilation issue.
• there is no workaround, the compilation plainly fail when the attribute is not registered.

See #917

[caridy]

I do not support this. Yes, from time to time, we need to add it (it is easy to add it to our map), but on the flip side, it captures so many pitfalls, so many silly mistakes made by developers, from typos to unnecessary/ignored attributes that will just add perf penalties.

We should quantify how many of those bugs are we getting, IMO, very small number so far, and go from there. Another alternative is to maybe try to use a test suite or something that will allow us to validate our compiler better.

[pmdartus]

I guess this is more of a linting problem, where we want to ensure that all the attributes are known in advance. However, it should not fail compilation.

Another important question is when should the attribute be added to this list of known attributes? For example, the intrinsicsize attribute on the <img> tag has been shipped in Chrome but not in other browsers. Should it be added to the template compiler?

[caridy]

yes, that second question is valid independent of the first question.

as for the first, either way, linting or compiler, it should fail, I don't think we can just pass them around as attributes for one reason:

<template>
     <div foo="something"></div>
     <x-bar foo="something"></div>
</template>

How are we going to reconcile these two tags if we allow arbitrary attributes to be placed everywhere? A hint here is that for regular elements they can be in attr, but for custom elements they can only be in props, how to reflect those?

[pmdartus]

I see what you mean here, in fact, it's a problem. That being said, I am impressed that we haven't had yet a lot of issues with attributes vs. props. Where developers would suspect that public properties on custom elements are reflected as attributes.

I guess one of the reason is probably that the template compiler doesn't allow to pass any unknown attribute. I am fine then with this restriction for now.

So what about the second question?

---

Tacking a step back from this thread, for non LWC custom element integration perspective, we will probably need a syntax in the future to differentiate between attributes and properties. The attr <-> prop reflection is only a best practice and not a requirement for the component author.

[kevinv11n]

Where developers would suspect that public properties on custom elements are reflected as attributes.

This has come up while we're writing tests with Dev Evangelism for all the sample repos. The developer expects the props they pass to a child to be accessible in a DOM query selector used in a test.

For example (source):

<template>
  <lightning-input name='firstName' label="First Name" onchange={handleChange}></lightning-input>
  <lightning-input name='lastName' label="Last Name" onchange={handleChange}></lightning-input>
</template>

name isn't reflected as an attribute so element.shadowRoot.querySelector("lightning-input[name='firstName']") doesn't work in tests or component code.

The latest guidance is to add a class or data- attribute, or select by tag name then check the props. Are we ok with that?

@trevor-bliss

[caridy]

reflection is coming! In fact, if you use the web component version of your lwc, attributes will be reflective, but props will not as today. There are three paths forward (btw, this is off topic, and probably deserves its own issue):

1. automatic reflection: reflect everything (a la react, which has perf implications, and you will see a lot of items="[object Object]" in the DOM.
2. mechanical reflection: dev specified reflection for some props (this is probably the sweet spot, and can be done in the @api decorator)
3. manual reflection
   :reflect some props manually (this is what we support today, devs will have to choose to reflect the normalize value in their setter)

For me, 1 is too much, it has perf implications, weirdness associate to it, etc. And 3 is too little, most people will never do that, too cumbersome. I'm ok with us trying to introduce 2 at some point, and keep 3 as the de-facto mechanism.

Btw, the fact that we don't allow CSS rules with custom attribute helps a lot, we could do the same with query selectors. I have an open issue to add more meaningful analysis of the queries passed to querySelector* APIs, and @davidturissini has some ideas how to make this happen, we can probably include custom attributes as well.

[pmdartus]

I have an open issue to add more meaningful analysis of the queries passed to querySelector* APIs

What do you want to extract from the querySelector*? If they are trying to access an attribute?

[diervo]

I still don't understand why we want to reflect attributes other than explicitly

[caridy]

What do you want to extract from the querySelector*? If they are trying to access an attribute?

In dev mode, we can find what attributes they are trying to filter by, and give them meaningful errors or warnings.

I still don't understand why we want to reflect attributes other than explicitly

@diervo I'm with you here. 2 and 3 are explicit, while 1 is implicit.

[pmdartus]

Property reflection is a nice to have feature. It requires today a lot of boilerplate code to reflect property to attribute, and there is no way today to reflect attribute to property.

You can find more detail here: https://github.com/salesforce/lwc/issues/896#issue-392533840

[caridy]

At this point, we are opting into removing the restriction entirely. @diervo convinced me that this risk is very little, and eventually we can add the reflection and not break anyone. If we do, then you can get @diervo to fix their code :)

[caridy]

@apapko @pmdartus @jodarove who can do this work? Btw, this is a blocker for SLDS v3

[jodarove]

@caridy i can work on this.

[pmdartus]

Sorry, @jodarove I didn't scroll down the issue. The PR is linked to the issue.

[caridy]

Ok, I think there are some confusion here, apologies for that. Let me clarify:

1. we should lift the throw in favor of a warning for regular elements with extraneous attributes.
2. we should keep the current behavior for those cases that we do not support, e.g.: is attribute is banned.
3. we should lift the restriction in CSS (css is the wild west). That's a different PR and issue altogether (@jodarove working on it).
4. we should keep the throw in CSS for LWC specific attributes (e.g.: key, lwc:*, etc.). That's a different PR and issue altogether (@jodarove working on it).

[jodarove]

fyi/ 3) and 4) is being addressed in https://github.com/salesforce/lwc/pull/1248