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

[LWC error]: Invalid event type #1811

Closed
jarrodek opened this issue Apr 7, 2020 · 12 comments
Closed

[LWC error]: Invalid event type #1811

jarrodek opened this issue Apr 7, 2020 · 12 comments
Labels

Comments

@jarrodek
Copy link

jarrodek commented Apr 7, 2020

Description

The compiler throws the following error:

[LWC error]: Invalid event type "value-changed" dispatched in element <awc-input>. Event name must start with a lowercase letter and followed only lowercase letters, numbers, and underscores

I am not sure why anyone decided to go around web standards and limited pattern for event names.

Steps to Reproduce

  1. Dispatch an event with a dash character.
this.dispatchEvent(new CustomEvent('value-changed', {
      detail: {
        value: 'test'
      }
}));

Expected Results

No error is throw, event is dispatched.

Actual Results

Error is thrown, event is not dispatched.

Browsers Affected

Chrome.

@diervo
Copy link
Contributor

diervo commented Apr 7, 2020

I think we did the restriction because the spec was somewhat ambiguous an there were some issues in safari. @pmdartus will remember better if we can lift this up.

@ekashida
Copy link
Member

ekashida commented Apr 7, 2020

We have this restriction because it allows users to listen for events both declaratively and imperatively while also following the web convention for event types.

If we wanted to add declarative support for value-changed then we would have to support onvalue-changed={handleValueChanged} in our template compiler which would introduce complexity because - implies camel-casing:

// js
@api
fooBar;

// html
<c-component foo-bar></c-component>

@jarrodek
Copy link
Author

jarrodek commented Apr 7, 2020

I am using this and other events like that cross-platform for a very long time and never had any problem. Although, I am not a fan of such naming right now (I can't recall any DOM event having a dash in a name). But If I am to rewrite quite complex application to LWC I prefer to have less to worry about.

I agree with the standards. But this standards is a recommendation. If that would be a standard then other options wouldn't be available in the web. Again, I completely agree to follow this recommendation. I fundamentally disagree with LWC team telling devs how to develop because you think you know better :) (I am writing this with a smile on my face so don't take this the wrong way).

As for this example from the previous post. That brings more questions. Why suddenly attributes are having dashes?!? The same standard is in the same table you referenced. There's a handful of attributes that have dashes in the names and only because they represents maps. Polymer team did it the right way in LitElement: lit/lit-element#29 (comment)

@jarrodek
Copy link
Author

jarrodek commented Apr 8, 2020

Is it possible to change the default behavior to not to use dashes in attribute names?

@diervo
Copy link
Contributor

diervo commented Apr 8, 2020

I recently found a quote that I really liked that embodies a response that fits very well here:

"I learned a painful lesson, that for small programs dynamic typing is great, for large programs you have to have a more disciplined approach and it helps if the language actually gives you that discipline, rather than telling you 'Well you can do whatever you want',"

This is Guido van Rossum, the creator of Python talking about Typescript.

The point I'm trying to make that hopefully you understand, is that in order for us to scale as a platform, and to allow thousands of developers from multiple companies to collaborate and thrive in a ecosystem for the long run with thousands of components working with each other, we must put a lot of rules and be opinionated: discipline.

Can you imagine if we were to do the same thing as lit, where people can create their own transformers at runtime? Just think the amount of devs that will copy paste some code (yes that what a lot of developers copy/paste code) - just look at that thread itself, the insane code that was pasted there with RegEx!), and think about the runtime implications of having 5000 components executing the same stupid code at runtime... All of this just because we "gave them the choice"? to which benefit really? Just so they can "choose" a way to shoot themselves in the foot?

I understand that a lot of developers won't understand this, and they want the freedom and the choice to do whatever they want, but, as the quote above states, it has a cost, a cost that in our business and our scale we can't afford to take. I been there, done that (ex. Aura), and suffered the consequences for years.

I can also understand how some developers for this very reason, won't like LWC or they would prefer an leaner and maybe simpler Web Component alternative like Lit (which is certainly an amazing library).

Most of the things we have done in LWC (full statically analyzability, component security, full declarative markup, wire protocol, avoid specific web component apis and primitives, ...) We have done for this very same reasoning.

And while we should always look for ways to make things simple and better, we must always be very careful of the implications of the choices we make when scale performance and ergonomics matter the most.

@ekashida
Copy link
Member

ekashida commented Apr 8, 2020

Sorry to hear that you are having a hard time with your rewrite.

It sounds like you prefer raw Web Components but I guess the reality is that every framework makes decisions around the sugar it introduces in its APIs. In the case of LWC, by following a convention where dash-delimited attribute names map to camel-cased property names, the component author doesn't have to spend so much time wiring up attributes and properties using low-level APIs like observedAttributes and attributeChangedCallback. As an added benefit, this design decision also introduces consistency across LWC components, as the foo-bar attribute always maps to the fooBar property, and vice-versa.

Is it possible to change the default behavior to not to use dashes in attribute names?

It's not possible to change the behavior but you could work around it by defining all your property names in lowercase. This should not be an issue as long as your property names are not ambiguous (e.g., fooBarBaz vs foobarBaz).

@jarrodek
Copy link
Author

jarrodek commented Apr 8, 2020

@diervo I hear you and completely agree. But how this answers the question why event's are limited this way and attributes are having special privilege of having dashes? This is literally the opposite of the point you are trying to make. The point of being opinionated it to embrace standards that you want the people to use. Then you have two different standards. This feels inconsistent and makes it even harder to understand the reasoning behind it. Similar example of that is not having hasAttribute while set/remove attribute functions are available. This is just bad developer experience so you shouldn't be surprised me asking hard questions when I don't see logic behind each limitation you add to the framework.

@ekashida I don't have problem with frameworks in general. I do prefer low level API as it gives you the most interoperability and and the lowest entry bar for people who don't know the framework. I was using Polymer since it's first release (was it 0.5 then?). Before that was Angular, GWT, and so on and so on. I was accepting limitations imposed on the development cycle back then. Even now I don't use "raw" components as that would be silly of me to play with attributes and life-cycle callbacks.

I also figured out that properties can all be lowercase then. But it is still inconsistent with the web platform! If it's inconsistent a developer has to learn how to use it properly. And if something is not intuitive then you have bumpkin like me asking stupid from your perspective questions. But I have different background and strong believe in using web standards first and then - and only if really necessary - some kind of abstraction.

@diervo
Copy link
Contributor

diervo commented Apr 8, 2020

This is literally the opposite of the point you are trying to make. The point of being opinionated it to embrace standards that you want the people to use.

Opinions and consistency are two different things :)

Sorry I don't understand, why you think attributes and events "are the same thing" or fall in the same bucket from a grammar restriction perspective? These are two orthogonal constructs. Its like you saying you don't want to have dashes for custom elements because you are used to elements like <input> because that's how you used to write HTML... Hence dashes are now inconsistent with everything else.

Eugene explained you that if we allow dashes it will be very weird to declaratively express events ex. onfoo-bar={handler}, looks misleading from an ergonomics perspective. If we allow dashes on events we then have to allow this, how do you feel about the ergonomics of it? Don't you think it will be even worst to allow you to add a dash in the event but then not let you declaratively add an event listener or do this weird alternative?

Moreover there are bugs on the HTML specification (I know because I did the specification for all the parser errors) and different interpretations that browsers do when it comes to uppercase/lowercase and non-alphanumeric characters. For making the HTML non-normative compliant we took the most conservative approach (subset of characters that the spec allows).

On the hasAttribute: Hasn't @caridy given you good reasoning and arguments behind hasAttribute removal?

Also, what is the thing that is non-standard here? We are restricting API surface. Unless my math very rusty, a subset of a standard still makes it standard...

@jarrodek
Copy link
Author

jarrodek commented Apr 8, 2020

We are still talking about two things.

I am not putting event's and attributes into the same bucket but they represent the same issue.
Web standard guidelines says to use lowercase, concatenated names. Not so surprisingly the same standard says the same about attributes. However LWC has one standard for one and different standard for another. There's no consistency. BTW, I am not the one who constantly repeats how LWC is consistent with web standards.

Eugene explained you that if we allow dashes it will be very weird to declaratively express events

No it's not: https://github.com/advanced-rest-client/api-request-editor/blob/stage/src/ApiRequestEditor.js#L1094

For making the HTML non-normative compliant we took the most conservative approach (subset of characters that the spec allows

And then somehow attributes gets be excluded? I am not sure what that means. Or are you referring to events only? How come it can work everywhere else but here?

On the hasAttribute: Hasn't @caridy given you good reasoning and arguments behind hasAttribute removal?

No, he hasn't. Not even close.

@caridy
Copy link
Contributor

caridy commented Apr 8, 2020

Ok, this is very dense at this point. Lets simplify it:

  1. LWC is an abstraction on top of HTMLElement, in that abstraction, we have made conscience decisions to restrict APIs and features to better preserve integrity of the platform.
  2. this "restrictions" are only applicable to our abstraction layer, which means the this value of a component that extends LightningElement, and the HTML, the css, the rest are just warnings.
  3. this helps the majority of our developers to produce something that can be used by other developers without surprise, and even beyond LWC, if you compile it to a web component, and use it from react, this restrictions do help (if you have followed https://custom-elements-everywhere.com/, you can see how it has been a long road to get everyone to interoperate).
  4. there is almost "always" a way around it, if you want to do your own thing, you can simply avoid the abstraction layer. In the case of the dispatchEvent, if you don't dispatch it on the component, but on another element from its shadow, e.g.: this.template.querySelector('div').dispatchEvent(...) do your thing.

If you feel strong that this is the wrong time of restriction, then try to make your case to demonstrate that this is affecting many users, what prior art exists in the DOM API that uses a dash in the name of the event, etc., so far this has only come up few times IIRC in the last few years. Be pragmatic, and try to understand the why.

If you don't find a proper workaround by not using the abstractions, then let us know, and we can try to provide an example.

@caridy caridy added the wontfix label Apr 8, 2020
@caridy
Copy link
Contributor

caridy commented Apr 8, 2020

For now, I'm closing the issue. Feel free to reopen it if you can provide the evidence and the details around it.

@caridy caridy closed this as completed Apr 8, 2020
@jarrodek
Copy link
Author

jarrodek commented Apr 8, 2020

Hey, thanks for detailed and good explanation @caridy It's all I needed to know in the first place :)
If that's the case I am OK with it. I put my fresh eyes on the API, felt it looks wrong and I reported it. As anyone else should.

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