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

Unnecessary event name restrictions (such as prohibiting hyphens) should be removed #1904

Closed
JanMiksovsky opened this issue May 28, 2020 · 10 comments

Comments

@JanMiksovsky
Copy link

Description

The LWC compiler currently imposes restrictions on the names of events which can be listened to through the on markup syntax. The restrictions seem unnecessary, limits developer control, and will unnecessarily constrain the interoperability of LWC with custom elements created by other frameworks.

Steps to Reproduce

Given a custom element sample-element that raises a CustomEvent with the valid name selected-index-change, attempt to instantiate the component in a template with:

<sample-element onselected-index-change={handleSelectedIndexChange}>

Expected Results

The code compiles, and listens to the selected-index-change event.

According to the MDN CustomEvent constructor docs, the type of an event is DOMString, i.e., a UTF-16 String. Since "selected-index-change" is such a string, the event name is legal.

Actual Results

The compiler generates an error:

[0] [!] (plugin rollup-plugin-lwc-compiler) Error: LWC1056:
Invalid event type "onselected-index-change". Event type must begin
with a lower-case alphabetic character and contain only lower-case
alphabetic characters, underscores, and numeric characters

Version

  • LWC: 1.3.7

Possible Solution

Relax artificial constraints on event names generally. To the extent possible, any valid DOMString should be permitted. In particular, while the native DOM events all are lowercase strings with no hyphens, a reasonable developer might decide they want to include hyphens in event names for legibility.

The fact that LWC templates are created in markup may some acceptable limitations. E.g., a single space character " " is a valid DOMString, but would presumably be problematic to parse in markup. It seems reasonable for a markup-based language to fail to parse such event names, and require use of addEventListener. Beyond that, the compiler should not impose additional artificial constraints.

@jarrodek
Copy link

This is related to: #1811

@caridy
Copy link
Contributor

caridy commented May 29, 2020

The gist of the issue:

<sample-element onselected-index-change={handleSelectedIndexChange} selected-index="1">

The question is:

  • what should be the event name considering that selected-index attribute will be set into selectedIndex property. Will the event name be selected-index-change or selectedIndexChange? That's the kind of ambiguity that we want to stay away from.

@jarrodek
Copy link

jarrodek commented May 29, 2020

That's the kind of ambiguity that we want to stay away from.

I believe that this is component's author decision to make and author's responsibility to document how to handle events. A component that author is developing is not a core platform feature. Because of that restrictions should be reduced to minimum.

@pmdartus
Copy link
Member

To the extent possible, any valid DOMString should be permitted.

The attribute name is not represented as a DOMString but a ASCII lowercase string, which restricts the character set that can be expressed in template. Because of this <sample-element onfooBar={handleEvent} onfoobar={handleEvent}> will both create the same event listener for
the foobar event.

That's the kind of ambiguity that we want to stay away from.

I believe that this is component's author decision to make and author's responsibility to document how to handle events. A component that author is developing is not a core platform feature. Because of that restrictions should be reduced to minimum.

The issue here is not on the component dispatching the event but on the consumer attempting to receive the event. For component properties, the attribute names written in kebab-cases are mapping to properties in camelCase. The question what is our stand regarding event listeners, does event listener name written in kebab-case are preserved or converted to camelCase.


Regardless of the outcome of the discussion, I would suggest updating the compiler error message to provide an alternative. What do you think about:

LWC1056: "onselected-index-change" is an ambiguous event listener name in the template. The ambiguity can be removed by attaching the event listener in you component via:
- either: this.addEventListener('select-index-change', /* handler */);
- or : this.addEventListener('selectIndexChange', /* handler */);

@jarrodek
Copy link

I am not sure if I understand this correctly. How select-index-change event corresponds to selectIndexChange? These are completely two different events. There is no ambiguity here.

@pmdartus
Copy link
Member

You are right those are 2 different event names. There is am ambiguity here since:

  • selectIndexChange can't be expressed via the template. The onselectIndexChange attribute name is equivalent to the onselectindexchange since all the attribute names when parsed from HTML are ASCI lowercased.
  • select-index-change is a little different. Today the template compiler transforms all the attribute names in kebad cases and map them to properties in camelCase in the recieving component. In the following example, <sample-element foo-bar> the foo-bar attribute maps to the fooBar property in the SampleElement LWC component. More details in the Set a Property on a Child Component section of the doc. Because of this what would the onselect-index-change event handler maps to? Does this event name maps to the select-index-change event (event name are untouched) or does it maps to the selectIndexChange (event name follow the same mapping convention than attribute to props).

@jarrodek
Copy link

Naturally it would correspond to select-index-change. It's not an attribute. It's an event registration. It can and should work differently to attributes so no one would confuse them.

@JanMiksovsky
Copy link
Author

JanMiksovsky commented May 29, 2020

Stepping back, I think the high-level question is: how many expectations should LWC place on the components used in an LWC app? For now, LWC isn't really optimized for consuming components from other frameworks, but it seems like the correct long-term goal is to work with web components created by any means.

With that in mind, the question for this specific issue becomes: what's the best job the LWC compiler can do to listen to events raised by any web component?

Per @pmdartus, the set of event names that can be practically listened to in markup is smaller than the set of allowable event names (which can be any string). I understand that the compiler today makes a simplifying assumption that all attributes can be handled the same way, but I agree with @jarrodek that events are not the same as properties, and deserve special handling.

As a starting point, following the principle of least surprise, it seems useful to say that someone should be able to listen to an event X using onX, with little or no translation of X by the compiler beyond what's necessary.

  • onfoo listens to foo
  • onfoo_change listens to foo_change
  • onfoo-change listens to foo-change
  • on変更 listens to 変更

I do not think it is surprising that onfoo-change listens to foo-change, even though foo-bar="value" invokes the fooBar property setter. It's just a guess, but I would hazard that hyphens in event names are more common than camel casing.

@pmdartus raises the point of casing. Given the general case-insensitive nature of markup, as a developer I would personally not be that surprised if

  • onfooBar listens to foobar

@diervo
Copy link
Contributor

diervo commented May 29, 2020

I can buy @JanMiksovsky articulation. Just to be clear the spec is somewhat ambiguous in the type of characters allowed, ambiguous meaning pretty much every character (but some unicode sequences) are allowed, where the reality is that browsers do have some constraints (ex. I believe the japanese characters did not work on safari).

I'm ok lifting this constraint for the sake of better WC interoperability and HTML declarative expressiveness, but we should add some guiding principle recommendation in the docs (like uppercasing is discouraged due to the ambiguity it represents).

@ekashida
Copy link
Member

This topic was discussed again in the context of improved integration support for third party web components. There was a general consensus that the usage of imperative API methods as a workaround for event types that LWC does not have declarative support for, is sufficient for now. It was also mentioned that if we do decide to introduce support at some point, it should come in the form of a new directive.

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

No branches or pull requests

6 participants