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

Use initCustomEvent instead of new CustomEvent #2101

Merged
merged 1 commit into from
Feb 19, 2019
Merged

Use initCustomEvent instead of new CustomEvent #2101

merged 1 commit into from
Feb 19, 2019

Conversation

Rich-Harris
Copy link
Member

This is one possible solution to #2018. It penalises non-IE11 users, which I don't like, but I'm not sure there's a clean alternative that doesn't involve polyfilling (which is a nuisance, even if the polyfill is small and simple).

Also, maybe it's better not to use legacy for this, since the other two places where it's used (for try-catching input.type = type, and replacing element.dataset.foo = bar with element.setAttribute('data-foo', bar)) are for IE9 (I think?), and it'd be nice if Svelte supported IE11 without the legacy flag since a lot of people are still in the unfortunate position of supporting it.

Any strong feelings either way?

@codecov-io
Copy link

codecov-io commented Feb 18, 2019

Codecov Report

Merging #2101 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #2101   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files           1       1           
  Lines          49      49           
======================================
  Hits           49      49

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 47ab23c...7c9c8ce. Read the comment docs.

@Conduitry
Copy link
Member

Yeah this seems reasonable. If the other legacy: true things are for even older versions of IE, it probably makes the most sense to just go with the IE11-compatible thing here. Requiring only transpiling and not polyfilling for IE11 sounds acceptable.

The cost thing this brings to even people not on IE11 is unfortunate, but slight. Even slighter if component events are less important in v3 than they were in v2.

@hontas
Copy link
Contributor

hontas commented Dec 22, 2019

What's unfortunate is that evt.initCustomEvent is not supported in Chrome versions prior to 59. Custom elements however are supported from Chrome 53. so this solution does not work for Chrome 53-59.

...yes I work on a project that has to support Chrome 53 😞 😢

To use new CustomEvent instead of document.createEvent('CustomEvent') and use the polyfill described here: https://developer.mozilla.org/en-US/docs/Web/API/CustomEvent/CustomEvent#Polyfill (247 bytes minified) would make more sense to me because because

  1. it is polyfillable
  2. it's not a deprecated api

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

Successfully merging this pull request may close these issues.

4 participants