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

Remove webcomponent polyfill from tests and only run custom element tests on browsers with native support #258

Closed
wants to merge 18 commits into from

Conversation

robframpton
Copy link

@robframpton robframpton commented Sep 14, 2017

From #230

Hi everyone,

This pull request aims to bring Custom element support for Metal components.

A new "package" named metal-custom-element has been introduced for this feature.

The code has been tested on Chrome (58.0.3029.81), Firefox (53.0), but also IE11 (11.0.0) and Microsoft Edge (14.14393.0) with ievms and the karam-ievms-launcher.

Here are a few notes that I wanted to share:

Two new babel "plugins" have been introduced in order to "transpile" correctly

https://github.com/github/babel-plugin-transform-custom-element-classes
https://www.npmjs.com/package/babel-plugin-transform-es2015-classes
The webcomponentjs polyfills are being used for browsers that don't natively support the different Web component APIs, and you'll notice that this dependency is being directly downloaded from the GitHub repository. The latest version is not published on npm and we're not using bower. An issue has been opened to get more information on when this package will be available through npm.

At the moment, nothing has been done concerning the Custom element adoptedCallback method, mostly for simplicity and also because this callback is not supported by the polyfill (see: https://github.com/webcomponents/custom-elements#known-bugs-and-limitations)

Concerning the tests:

The polyfills used, are (or must be) doing something to HTMLElement (or Node) because they cause a test that was working to break

This is the specific test, and here are some of the error messages in different browsers

Firefox: Firefox 53.0.0 (Mac OS X 10.12.0) dom manipulation should append node list to parent element FAILED
Argument 1 of Node.appendChild does not implement interface Node.
Edge: Edge 14.14393.0 (Windows 10 0.0.0) dom manipulation should append node list to parent element FAILED
Error: No such interface supported
IE11: IE 11.0.0 (Windows 7 0.0.0) dom manipulation should append node list to parent element FAILED
HierarchyRequestError
Concerning IE10:

I have not been able to run the tests in IE10 despite multiple attempts. The "browserified" bundle loaded by Karma, launches an error in IE10, concerning Object.setPrototypeOf.

The polyfill used do no support IE10 either.

I am trying to find a way to "skip" the tests, as well as not loading the bundled polyfills for IE10 and lower, but have not yet found a way. - Any help is welcome

As usual your ideas and feedback concerning improvements are welcome.

Thanks!

@robframpton
Copy link
Author

Hey @jbalsas @julien

As you can see by the tests this works on very little browsers without the polyfill.

@jbalsas
Copy link
Contributor

jbalsas commented Sep 19, 2017

Hey @Robert-Frampton, we still have several browsers failing here... maybe a side effect of the PR?

This is the current support table for Custom Elements basic support:

Firefox (Gecko) Chrome Internet Explorer Edge Opera Safari
behind dom.webcomponents.enabled? 59.0 No support No support 47.0 10.1

Are you mentioning it because you think we shouldn't push this in?

@robframpton
Copy link
Author

Yes I'm aware of the failing tests, and yes I mentioned it because I'm really unsure if this feature is worth pursuing.

IMHO these are our options.

  1. Address the incompatibility of metal-dom and the web components polyfill and release this with instructions on how to include it. It's just a single method that is getting clobbered, so I think it's doable.

  2. Just forget about releasing this feature. Without the polyfill it's barely useful, and I just don't see people using it.

@jbalsas what do you think?

@jbalsas
Copy link
Contributor

jbalsas commented Sep 19, 2017

Hey @Robert-Frampton, I have the feeling it is more useful that we might think... for instance, we have some users with a more restricted browser matrix (see Screens, or hybrid mobile) where just having this with Chrome Support would be the selling point to use metal.js as a base framework...

If 1 is doable, I would try it. Polymer is moving in this direction and I feel a stronger push this time in general. Sure, it is still a bit in the works, but we have a chance to get on board sooner rather than later, and we don't need to go all in, just have some nice story around it.

@robframpton robframpton force-pushed the webcomponents branch 3 times, most recently from 1250d9f to e4ca863 Compare September 27, 2017 19:41
@robframpton
Copy link
Author

#261

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.

2 participants