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

✨ Enable native Custom Elements v1 #25173

Merged
merged 4 commits into from
Oct 23, 2019

Conversation

jridgewell
Copy link
Contributor

This will attempt to use native CEv1 when supported.

Based on whether the anonymous class remains a real, native ES6 class, we can use 0 polyfill (fully native support). If it's transpiled into a function () {}, then we can use a minimal polyfill that wraps the HTMLElement constructor only.

This will attempt to use native CEv1 when supported. Based on whether
the anonymous class remains a real, native ES6 class, we can use 0
polyfill (fully native support). If it's transpiled into a `function ()
{}`, then we can use a minimal polyfill that wraps the `HTMLElement`
constructor only.
@jridgewell jridgewell force-pushed the custom-elements-v1-native branch from e24c911 to 0e9cf82 Compare October 22, 2019 18:35
@@ -377,6 +377,8 @@ describe('amp-img', () => {
it('should propagate the object-fit attribute', () => {
return getImg({
src: '/examples/img/sample.jpg',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were these test breaking due to this change (if so why?)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the native Custom Elements implementations are able to fire connectedCallback much quicker than the MutationObserver the polyfill uses.

@jridgewell
Copy link
Contributor Author

I created a new RTV experiment so that this can be tested on a smaller population.

/cc @lannka and @zhouyx, I also updated the .eslintrc so that RTV experiments are recognized as globals. It'd be really convenient if we could change to a .eslintrc.js file, so that we could dynamically generate the defined experiment globals from experiments-config.json. @rsimha?

@zhouyx
Copy link
Contributor

zhouyx commented Oct 23, 2019

I also updated the .eslintrc so that RTV experiments are recognized as globals. It'd be really convenient if we could change to a .eslintrc.js file, so that we could dynamically generate the defined experiment globals from experiments-config.json. @rsimha?

Sounds like a great idea!

Copy link
Contributor

@rsimha rsimha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be really convenient if we could change to a .eslintrc.js file, so that we could dynamically generate the defined experiment globals from experiments-config.json. @rsimha?

This might be possible, but will require lots of testing with our existing lint toolchain, VS Code, Prettier, etc. Can you file a FR?

.eslintrc Show resolved Hide resolved
@jridgewell jridgewell merged commit 53a7228 into ampproject:master Oct 23, 2019
@jridgewell jridgewell deleted the custom-elements-v1-native branch October 23, 2019 23:03
micajuine-ho pushed a commit to micajuine-ho/amphtml that referenced this pull request Dec 27, 2019
* Try enabling native Custom Elements v1

This will attempt to use native CEv1 when supported. Based on whether
the anonymous class remains a real, native ES6 class, we can use 0
polyfill (fully native support). If it's transpiled into a `function ()
{}`, then we can use a minimal polyfill that wraps the `HTMLElement`
constructor only.

* Fix img tests

* Bump travis

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

Successfully merging this pull request may close these issues.

6 participants