✨🏗 Upgrade Closure Compiler to v20180101 by rsimha · Pull Request #18794 · ampproject/amphtml

rsimha · 2018-10-17T21:48:18Z

It's 2018, so this PR upgrades closure compiler to the earliest version from 2018.

Code changes:

Undo the promise-pjs workaround introduced by @cramforce in Turn on closure compiler collapse properties. #2972.
Fix the Property so-and-so never defined errors in gulp check-types by modifying a few imports
Unset the process_common_js_modules option for single-pass to prevent errors due to class exports being appended with .default. To make up for it, also do the following:
- Apply the babel-plugin-transform-commonjs-es2015-modules transform to common JS dependencies that are shipped with the runtime (promise-pjs,dompurify, and set-dom)
- Remove the UMD wrapper from third_party/mustache.js to make it an ESM style module
- Replace the common JS version of @ampproject/animations/dist/animations that we were using in amp-lightbox-gallery with its ESM equivalent
Fix a class redefinition error in react-utils.js by renaming Deferred to DeferredType (Deferred is already defined and exported by src/promise.js)
Fix a redefinition error of width, left, and right in amp-image-slider.js

Coming up: More PRs to eventually upgrade to the latest version.

Partial fix for #18748
Follow up to #18552
Follow up to #18609

rsimha · 2018-10-17T21:48:42Z

/to @choumx @erwinmombay @jridgewell @kristoferbaxter

rsimha · 2018-11-07T16:24:19Z

This PR is now ready for review.

Adding @cvializ to review the change to react-utils.js.

cvializ

react-utils changes look good 👍 "deferring" approval to other reviewers : )

dreamofabear · 2018-11-08T19:53:44Z

Please ping on this PR when it's tested and ready.

rsimha · 2018-11-12T23:17:02Z

@choumx Pinging this PR. To get the runtime to work in single-pass mode, I had to do a couple things like transform common JS dependencies to ESM and remove the UMD wrapper from third_party/mustache.js. Sending this PR out for a fresh review, so we can figure out which of these changes are acceptable, and what we must do in place of the changes that aren't.

third_party/mustache/mustache.js

rsimha · 2018-11-14T22:21:08Z

@choumx @jridgewell @erwinmombay At long last, this is ready for review. PTAL.

rsimha · 2018-11-15T18:26:15Z

Tested this PR as follows:

Double checked that the workflow I used to upgrade closure was correct
Made sure all Travis checks passed
Compiled runtime locally in multi-pass and single-pass modes
Served runtime locally in both modes and made sure several example pages load and render correctly
Ran integration tests on latest linux Chrome in compiled mode

Merging now 🤞

kristoferbaxter · 2018-11-15T18:32:17Z

build-system/build.conf.js

-    return defaultPlugins;
+    if (isCommonJsModule) {
+      pluginsToApply = pluginsToApply.concat([
+        [require.resolve('babel-plugin-transform-commonjs-es2015-modules')],


Do we have an open issue for removing this transform when possible? We should migrate over all these dependencies to ESM.

I have a TODO in the code assigned to @erwinmombay and me. I'm working on a follow up PR to upgrade to the latest closure as we speak, and will file issues for all remaining ESM library related tasks.

Makes sense, thanks @rsimha.

Sent out #23106 to clean this up.

build-system/single-pass.js

rsimha added the WG: infra label Oct 17, 2018

rsimha self-assigned this Oct 17, 2018

rsimha requested review from dreamofabear and jridgewell October 17, 2018 21:48

googlebot added the cla: yes label Oct 17, 2018

rsimha requested a review from erwinmombay October 17, 2018 21:48

rsimha requested a review from cvializ November 7, 2018 16:23

rsimha requested a review from kristoferbaxter November 7, 2018 16:27

cvializ reviewed Nov 7, 2018

View reviewed changes

rsimha mentioned this pull request Nov 7, 2018

🏗✨ Move rrule from third_party to node_modules #19177

Closed

rsimha commented Nov 12, 2018

View reviewed changes

third_party/mustache/mustache.js Show resolved Hide resolved

rsimha added 10 commits November 14, 2018 17:12

Upgrade closure compiler to v20180101

3409ad1

Fix build with new closure compiler

6f99682

Transform common JS modules to ESM

e4ea571

Fix multi-pass compile

5ec917a

Convert third_party/mustache.js from UMD to ESM

28155e4

Use ESM version of @ampproject/animations

b0a8d65

Convert set-dom to ESM

dfb49f9

Add documentation

a10a57f

Document changes to third_party/mustache and automate future updates

618c7a3

Fix lint

129d83c

erwinmombay approved these changes Nov 15, 2018

View reviewed changes

rsimha merged commit b6b8129 into ampproject:master Nov 15, 2018

rsimha deleted the 2018-10-15-ClosureUpgrade branch November 15, 2018 18:27

kristoferbaxter reviewed Nov 15, 2018

View reviewed changes

ampprojectbot requested review from cathyxz, cramforce, cvializ, dvoytenko, erwinmombay and sparhami November 15, 2018 18:33

kristoferbaxter reviewed Nov 15, 2018

View reviewed changes

build-system/single-pass.js Show resolved Hide resolved

ampproject deleted a comment from ampprojectbot Nov 15, 2018

rsimha mentioned this pull request Nov 16, 2018

🏗 Update third_party/closure-library/sha384-generated.js #19356

Merged

ampprojectbot added the PR use: In Beta / Experimental label Nov 21, 2018

rsimha mentioned this pull request Nov 26, 2018

✨🏗 Upgrade closure compiler to v20181028 #19449

Closed

Enriqe pushed a commit to Enriqe/amphtml that referenced this pull request Nov 28, 2018

✨🏗 Upgrade Closure Compiler to v20180101 (ampproject#18794)

69c34fd

ampprojectbot added the PR use: In Stable label Nov 28, 2018

jeffkaufman mentioned this pull request Dec 5, 2018

Broken cross-domain check on Inabox #19653

Closed

rsimha mentioned this pull request Dec 21, 2018

✨🏗 [WIP] Upgrade closure compiler to v20181210 #20056

Closed

rsimha mentioned this pull request Mar 28, 2019

✨🏗 Upgrade closure compiler to v20190301 #21618

Merged

rsimha mentioned this pull request May 22, 2019

🏗✨ Upgrade closure compiler to v20190513 #22446

Merged

rsimha mentioned this pull request Jun 28, 2019

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

Merged

rsimha mentioned this pull request Jul 18, 2019

🏗✨ Upgrade closure compiler to v20190709 #23417

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨🏗 Upgrade Closure Compiler to v20180101#18794

✨🏗 Upgrade Closure Compiler to v20180101#18794
rsimha merged 10 commits intoampproject:masterfrom
rsimha:2018-10-15-ClosureUpgrade

rsimha commented Oct 17, 2018 •

edited

Loading

Uh oh!

rsimha commented Oct 17, 2018 •

edited

Loading

Uh oh!

rsimha commented Nov 7, 2018

Uh oh!

cvializ left a comment

Uh oh!

dreamofabear commented Nov 8, 2018

Uh oh!

rsimha commented Nov 12, 2018

Uh oh!

Uh oh!

rsimha commented Nov 14, 2018

Uh oh!

rsimha commented Nov 15, 2018 •

edited

Loading

Uh oh!

kristoferbaxter Nov 15, 2018

Uh oh!

rsimha Nov 15, 2018

Uh oh!

kristoferbaxter Nov 15, 2018

Uh oh!

rsimha Jun 28, 2019

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

rsimha commented Oct 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rsimha commented Oct 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rsimha commented Nov 7, 2018

Uh oh!

cvializ left a comment

Choose a reason for hiding this comment

Uh oh!

dreamofabear commented Nov 8, 2018

Uh oh!

rsimha commented Nov 12, 2018

Uh oh!

Uh oh!

rsimha commented Nov 14, 2018

Uh oh!

rsimha commented Nov 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kristoferbaxter Nov 15, 2018

Choose a reason for hiding this comment

Uh oh!

rsimha Nov 15, 2018

Choose a reason for hiding this comment

Uh oh!

kristoferbaxter Nov 15, 2018

Choose a reason for hiding this comment

Uh oh!

rsimha Jun 28, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

rsimha commented Oct 17, 2018 •

edited

Loading

rsimha commented Oct 17, 2018 •

edited

Loading

rsimha commented Nov 15, 2018 •

edited

Loading