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 workaround to transform Common JS modules to ESM #23106

Merged
merged 3 commits into from Jul 2, 2019
Merged

🏗🚮 Remove workaround to transform Common JS modules to ESM #23106

merged 3 commits into from Jul 2, 2019

Conversation

rsimha
Copy link
Contributor

@rsimha rsimha commented Jun 28, 2019

During a previous closure compiler upgrade (#18794), we had to apply a workaround to transform a few modules used by the AMP runtime from Common JS to ESM. This was needed because single pass compilation was broken by the closure flag process_common_js_modules.

The workaround is no longer necessary because all three modules now have ESM equivalents, and we no longer need to transform them.

This PR does the following:

  • Moves the AMP runtime's existing usage of Common JS modules to their ESM equivalents
    • dompurify: Already using ESM
    • promise-pjs: Switched from v1.1.3 to v1.1.4, which supports ESM
    • set-dom: Changed usage from Common JS to ESM
  • Removes all code in single-pass.js and build.conf.js that has to do with Common JS -> ESM transformation
  • Removes babel-plugin-transform-commonjs-es2015-modules

Addresses #18794 (review)

@rsimha rsimha requested a review from dreamofabear July 1, 2019 22:18
@rsimha rsimha merged commit 4b86736 into ampproject:master Jul 2, 2019
@rsimha rsimha deleted the 2019-06-28-CommonJsModules branch July 2, 2019 17:02
@dreamofabear
Copy link

Nice.

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

6 participants