✨🏗 Upgrade Closure Compiler to v20180101 #18794

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 jridgewell and dreamofabear 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 commented Oct 17, 2018 •

edited

rsimha commented Oct 17, 2018 •

edited

rsimha commented Nov 7, 2018

cvializ left a comment

dreamofabear commented Nov 8, 2018

rsimha commented Nov 12, 2018

rsimha commented Nov 14, 2018

rsimha commented Nov 15, 2018 •

edited

kristoferbaxter Nov 15, 2018

rsimha Nov 15, 2018

kristoferbaxter Nov 15, 2018

rsimha Jun 28, 2019

✨🏗 Upgrade Closure Compiler to v20180101 #18794

✨🏗 Upgrade Closure Compiler to v20180101 #18794

Conversation

rsimha commented Oct 17, 2018 • edited

rsimha commented Oct 17, 2018 • edited

rsimha commented Nov 7, 2018

cvializ left a comment

Choose a reason for hiding this comment

dreamofabear commented Nov 8, 2018

rsimha commented Nov 12, 2018

rsimha commented Nov 14, 2018

rsimha commented Nov 15, 2018 • edited

kristoferbaxter Nov 15, 2018

Choose a reason for hiding this comment

rsimha Nov 15, 2018

Choose a reason for hiding this comment

kristoferbaxter Nov 15, 2018

Choose a reason for hiding this comment

rsimha Jun 28, 2019

Choose a reason for hiding this comment

rsimha commented Oct 17, 2018 •

edited

rsimha commented Oct 17, 2018 •

edited

rsimha commented Nov 15, 2018 •

edited