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 custom-elements and IntersectionObserver polyfill for SxG. #29971

Merged
merged 7 commits into from Aug 27, 2020

Conversation

xiexr151e
Copy link
Contributor

@xiexr151e xiexr151e commented Aug 25, 2020

@jridgewell mentioned that the custom-elements polyfill could be removed for the SxG build. This PR is a one line change that achieves that.
In addition, the IntersectionObserver polyfill can also be removed as mentioned by @erwinmombay.

@google-cla google-cla bot added the cla: yes label Aug 25, 2020
@xiexr151e xiexr151e marked this pull request as ready for review August 25, 2020 20:05
Copy link
Contributor

@jridgewell jridgewell left a comment

Choose a reason for hiding this comment

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

We also need to strip in in src/friendly-iframe-embed-legacy.js and src/friendly-iframe-embed.js

@xiexr151e
Copy link
Contributor Author

@jridgewell Kris mentioned that IntersectionObserver may also not be needed in SxG, should I just remove that in this PR also?

@xiexr151e xiexr151e changed the title 🚀 Remove custom-elements polyfill for SxG. 🚀 Remove custom-elements and IntersectionObserver polyfill for SxG. Aug 25, 2020
@xiexr151e
Copy link
Contributor Author

We also need to strip in in src/friendly-iframe-embed-legacy.js and src/friendly-iframe-embed.js

Just finished.

@kevinkimball
Copy link
Contributor

@xiexr151e What is the size reduction?

@xiexr151e
Copy link
Contributor Author

xiexr151e commented Aug 25, 2020

@xiexr151e What is the size reduction?

1.96KB, after comparing ESM to SxG binary size.

Copy link
Contributor

@kevinkimball kevinkimball left a comment

Choose a reason for hiding this comment

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

LGTM

src/friendly-iframe-embed-legacy.js Outdated Show resolved Hide resolved
src/friendly-iframe-embed.js Outdated Show resolved Hide resolved
@xiexr151e xiexr151e merged commit d0c677d into ampproject:master Aug 27, 2020
@xiexr151e xiexr151e deleted the derek-custom-ele branch August 27, 2020 04:56
ed-bird pushed a commit to ed-bird/amphtml that referenced this pull request Dec 10, 2020
…mpproject#29971)

* remove custom elements polyfill

* undo viewport

* remove install in embed

* remove IntersectionObserver

* lint

* remove 2 polyfills for esm/sxg

Co-authored-by: Derek Tse <derektse@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants