-
Notifications
You must be signed in to change notification settings - Fork 25
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
rfc: declarative binding for non-standard event names #88
base: master
Are you sure you want to change the base?
Conversation
12396e4
to
491504f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great exploratory RFC, but I think we should pin it down to a particular proposal and evaluate the pros/cons of that. There are a few different options being weighed here, but I think the onFoo-BarBaz
variant is the most viable, to me anyway.
<third-party lwc:external lwc:on:Foo-Bar-BAZ={handleConnected}></third-party> | ||
|
||
<!-- Event as part of attribute name (special treatment for lwc:external) --> | ||
<third-party lwc:external onFoo-Bar-BAZ={handleConnected}></third-party> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To me, this option is the least disruptive. Yes, it is playing fast-and-loose with HTML semantics, but it is the most similar to what we have today, avoiding the need for a special new lwc:on
directive or other mechanism.
Scoping it to lwc:external
also brings that advantage that we can pass 100% on Custom Elements Everywhere without changing how LWC components have worked up until today. (We can re-evaluate that later if needed, but I really think it's not a big deal – the important thing is just to support the wide world of third-party custom elements and their wacky naming conventions for events.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that it's the least disruptive, but I also feel like it might be a source of confusion/surprise. The nice thing about introducing a new directive is that it's easy to understand...if you want to do this thing that you were never able to before, you have to use this new directive.
PascalCase: (event) => console.log("'PascalCase' handled"), | ||
}; | ||
} | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This to me is unnecessary since we already have lwc:spread
, which works with onclick
/onblur
/etc. We may have to consider how lwc:spread
would interact, though, with lwc:external
if we decided to support the non-standard event names there.
used to listen for standard events like `click` or `focus` by assigning event listeners to the | ||
`onclick` and `onfocus` properties. However, adding support for non-standard event names would add | ||
complexity to `lwc:spread`. Introducing a new directive also makes it easier to restrict the usage | ||
of `lwc:on` to `lwc:external` components. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It certainly adds complexity if we want to maintain the difference between lwc:external
vs LWC components. We'd have to add validation to the client side (runtime) as well as at compile-time. Although we could just not support lwc:spread
with non-standard event names entirely – we'd still score 100% on Custom Elements Everywhere, so the value is debatable.
|
||
## lwc:on=`${eventName}:${listenerName}` | ||
|
||
Encoding both the event name and event handler name as a string using some delimiter would allow us |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a big fan of lwc:on
, especially since Svelte v5 is actually moving away from their existing on:click
syntax towards a syntax more like ours, since it plays nicer with their equivalent of lwc:spread
.
This is not desirable because it makes static analysis more difficult. Conversely, consumers might | ||
use this directive when they could just as easily have used a standard `on*` attribute in their | ||
template. These scenarios could be avoided, however, by restricting the usage of `lwc:on` to custom | ||
elements that use the `lwc:external` directive. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lwc can be parent or owner of components using lwc:external
. And these events could reach them if bubbles
or composed
is true
. Do we not want to support those cases?
Rendered