Conversation
KeithHenry
left a comment
There was a problem hiding this comment.
I think this would break React style camelCase events (like onClick).
|
@KeithHenry you're right! not sure what I was thinking there. As for other options, maybe checking for the existence of an if (name.toLowerCase() in node) name = name.toLowerCase(); |
|
I was thinking about this today after Rob Dodson's talk on Polymer summit. The only way I can imagine this to work is if you have a whilelist of native DOM events and bail from lowercase if the string is not found. Sad because this would increase lib size. |
|
@jeremenichelli you don't think my |
|
@developit can you point the line you are doing the The issue here I think is the fact that after L67 we need to somehow know if the event name needs to be normalized to lowercase, before the listener is added. I don't know for sure how safe this approach is but checking the full event name against the body object might help, somethink like: |
|
@jeremenichelli - it wasn't in this PR, was just an idea I came up with. It would look very similar to what you suggested, but rather than using let nameLower = name.toLowerCase();
if (node[nameLower]!=undefined) {
name = nameLower;
}
name = name.substring(2); |
|
Oh I get it now @developit. Let me know if there's anything I can help with, really interested in helping to push this forward 🎉 |
|
Same here! I'll update the PR with the fix and we can see what |
|
Looks cool, would be nice to add tests for custom elements event if current Preact's test suit allows it. If not it would be a nice future PR. |
|
Yup. We'll need to switch the tests from running in PhantomJS to Chrome Headless, since Phantom doesn't support web components. |
|
@developit Was this blocked at some point since the analysis on what this change cost as far as |
|
btw I've been meaning to write a blog post on this... In building custom-elements-everywhere, I've seen issues in various libraries around mixed case events. I'm increasingly of the opinion that, as a best practice, event names should just be all one word, all lowercase. That's how the platform handles things today: By sticking to all lowercase, all one-word, you free up the libraries from each needing to invent their own heuristic to grok what event name you were actually trying to use. |
|
@lozandier yup - we can't do whitelists, so detection is the only way to go here, and I'm not sure that can even work. Maybe we'll get browsers to switch those last few event types to lowercase ;) |
|
Fixed in X. |
No description provided.