Roll forward CC upgrade with fixes (ampproject#18552) (ampproject#18609)

* Revert "Revert CC upgrade. (ampproject#18600)" This reverts commit 6fd3044. * Remove node_modules/ from js_module_root. * Fix swg.js again. * Increase bundle size cap to 82.2. * Inject react-dates bundle post-compile instead of importing in code. * Typecast react-dates imports in amp-date-picker. * Bump bundle-size. * Restore --js_module_root for 1-pass. * Remove obsolete 'erwinmHack'.
Enriqe · Nov 28, 2018 · 463f8f1 · 463f8f1
1 parent 4a26a55
commit 463f8f1
Show file tree

Hide file tree

Showing 58 changed files with 672 additions and 699 deletions.
diff --git a/ads/adventive.js b/ads/adventive.js
@@ -139,10 +139,13 @@ function getUrl(context, data, ns) {
  *    representation of the url search query.
  */
 function reduceSearch(ns, placementId, click, referrer) {
-  const isRecipeLoaded = hasOwn(ns.args, 'placementId'),
-      isRecipeStale = !isRecipeLoaded ? true :
-        (Date.now() - ns.args[placementId].requestTime) > (60 * cacheTime),
-      needsRequest = !isRecipeLoaded || isRecipeStale;
+  const isRecipeLoaded = hasOwn(ns.args, 'placementId');
+  let isRecipeStale = true;
+  if (isRecipeLoaded) {
+    const info = ns.args[placementId];
+    isRecipeStale = (Date.now() - info['requestTime']) > (60 * cacheTime);
+  }
+  const needsRequest = !isRecipeLoaded || isRecipeStale;
 
   return !needsRequest ? null : dict({
     'click': click,

diff --git a/build-system/get-dep-graph.js b/build-system/get-dep-graph.js
@@ -82,11 +82,16 @@ exports.getFlags = function(config) {
     language_in: 'ES6',
     language_out: config.language_out || 'ES5',
     module_output_path_prefix: config.writeTo || 'out/',
+    module_resolution: 'NODE',
     externs: config.externs,
     define: config.define,
     // Turn off warning for "Unknown @define" since we use define to pass
     // args such as FORTESTING to our runner.
     jscomp_off: ['unknownDefines'],
+    // checkVars: Demote "variable foo is undeclared" errors.
+    // moduleLoad: Demote "module not found" errors to ignore missing files
+    //     in type declarations in the swg.js bundle.
+    jscomp_warning: ['checkVars', 'moduleLoad'],
     jscomp_error: [
       'checkTypes',
       'accessControls',

diff --git a/build-system/runner/dist/runner.jar b/build-system/runner/dist/runner.jar
diff --git a/build-system/runner/src/org/ampproject/AmpCommandLineRunner.java b/build-system/runner/src/org/ampproject/AmpCommandLineRunner.java
@@ -80,8 +80,6 @@ protected AmpCommandLineRunner(String[] args) {
     options.setDevirtualizePrototypeMethods(true);
     options.setExtractPrototypeMemberDeclarations(true);
     options.setSmartNameRemoval(true);
-    options.optimizeParameters = true;
-    options.optimizeReturns = true;
     options.optimizeCalls = true;
     if (single_file_compilation) {
       options.renamePrefixNamespace = "_";

diff --git a/build-system/runner/src/org/ampproject/AmpPass.java b/build-system/runner/src/org/ampproject/AmpPass.java
@@ -158,7 +158,7 @@ private void inlineAmpExtensionCall(Node n, Node expr) {
     newcall.putBooleanProp(Node.FREE_CALL, true);
     newcall.addChildToBack(arg1);
     expr.replaceChild(n, newcall);
-    compiler.reportCodeChange();
+    compiler.reportChangeToEnclosingScope(expr);
   }
 
   private Node getAmpExtensionCallback(Node n) {
@@ -226,7 +226,7 @@ private void maybeReplaceRValueInVar(Node n, Map<String, Node> map) {
         for (Map.Entry<String, Node> mapping : map.entrySet()) {
           if (varNode.getString() == mapping.getKey()) {
             varNode.replaceChild(varNode.getFirstChild(), mapping.getValue());
-            compiler.reportCodeChange();
+            compiler.reportChangeToEnclosingScope(varNode);
             return;
           }
         }
@@ -276,17 +276,19 @@ private void replaceWithBooleanExpression(boolean bool, Node n, Node parent) {
     Node booleanNode = bool ? IR.trueNode() : IR.falseNode();
     booleanNode.useSourceInfoIfMissingFrom(n);
     parent.replaceChild(n, booleanNode);
-    compiler.reportCodeChange();
+    compiler.reportChangeToEnclosingScope(parent);
   }
 
   private void removeExpression(Node n, Node parent) {
+    Node scope = parent;
     if (parent.isExprResult()) {
       Node grandparent = parent.getParent();
       grandparent.removeChild(parent);
+      scope = grandparent;
     } else {
       parent.removeChild(n);
     }
-    compiler.reportCodeChange();
+    compiler.reportChangeToEnclosingScope(scope);
   }
 
   private void maybeEliminateCallExceptFirstParam(Node call, Node parent) {
@@ -306,7 +308,7 @@ private void maybeEliminateCallExceptFirstParam(Node call, Node parent) {
 
     firstArg.detachFromParent();
     parent.replaceChild(call, firstArg);
-    compiler.reportCodeChange();
+    compiler.reportChangeToEnclosingScope(parent);
   }
 
   /**

diff --git a/build-system/runner/test/org/ampproject/AmpPassTest.java b/build-system/runner/test/org/ampproject/AmpPassTest.java
@@ -8,15 +8,15 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.javascript.jscomp.Compiler;
 import com.google.javascript.jscomp.CompilerPass;
-import com.google.javascript.jscomp.Es6CompilerTestCase;
+import com.google.javascript.jscomp.CompilerTestCase;
 import com.google.javascript.rhino.IR;
 import com.google.javascript.rhino.Node;
 
 
 /**
  * Tests {@link AmpPass}.
  */
-public class AmpPassTest extends Es6CompilerTestCase {
+public class AmpPassTest extends CompilerTestCase {
 
   ImmutableMap<String, Set<String>> suffixTypes = ImmutableMap.of(
       "module$src$log.dev",
@@ -305,7 +305,7 @@ public void testGetModePreserve() throws Exception {
   }
 
   public void testOptimizeGetModeFunction() throws Exception {
-    testEs6(
+    test(
         LINE_JOINER.join(
              "(function() {",
              "const IS_DEV = true;",
@@ -321,7 +321,7 @@ public void testOptimizeGetModeFunction() throws Exception {
   }
 
   public void testRemoveAmpAddExtensionCallWithExplicitContext() throws Exception {
-    testEs6(
+    test(
         LINE_JOINER.join(
             "var a = 'hello';",
             "self.AMP.extension('hello', '0.1', function(AMP) {",
@@ -339,7 +339,7 @@ public void testRemoveAmpAddExtensionCallWithExplicitContext() throws Exception
   }
 
   public void testRemoveAmpAddExtensionCallWithNoContext() throws Exception {
-    testEs6(
+    test(
         LINE_JOINER.join(
             "var a = 'hello';",
             "AMP.extension('hello', '0.1', function(AMP) {",

diff --git a/build-system/runner/test/org/ampproject/AmpPassTestEnvTest.java b/build-system/runner/test/org/ampproject/AmpPassTestEnvTest.java
@@ -6,16 +6,15 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.javascript.rhino.IR;
 import com.google.javascript.rhino.Node;
-import com.google.common.collect.ImmutableSet;
 import com.google.javascript.jscomp.Compiler;
 import com.google.javascript.jscomp.CompilerPass;
-import com.google.javascript.jscomp.Es6CompilerTestCase;
+import com.google.javascript.jscomp.CompilerTestCase;
 
 
 /**
  * Tests {@link AmpPass}.
  */
-public class AmpPassTestEnvTest extends Es6CompilerTestCase {
+public class AmpPassTestEnvTest extends CompilerTestCase {
 
   ImmutableMap<String, Set<String>> suffixTypes = ImmutableMap.of();
   ImmutableMap<String, Node> assignmentReplacements = ImmutableMap.of(
@@ -97,7 +96,7 @@ public void testGetModeMinifiedPropertyReplacementInTestingEnv() throws Exceptio
   }
 
   public void testOptimizeGetModeFunction() throws Exception {
-    testEs6(
+    test(
         LINE_JOINER.join(
              "(function() {",
              "const IS_DEV = true;",

diff --git a/build-system/tasks/bundle-size.js b/build-system/tasks/bundle-size.js
@@ -22,7 +22,7 @@ const log = require('fancy-log');
 const {getStdout} = require('../exec');
 
 const runtimeFile = './dist/v0.js';
-const maxSize = '81.9KB'; // Only use 0.1 KB of precision (no hundredths digit)
+const maxSize = '82.4KB'; // Only use 0.1 KB of precision (no hundredths digit)
 
 const {green, red, cyan, yellow} = colors;
 

diff --git a/build-system/tasks/compile.js b/build-system/tasks/compile.js
@@ -54,7 +54,7 @@ exports.closureCompile = function(entryModuleFilename, outputDir,
             next();
             resolve();
           }, function(e) {
-            console./* OK*/error(colors.red('Compilation error:', e.message));
+            console./*OK*/error(colors.red('Compilation error:'), e.message);
             process.exit(1);
           });
     }
@@ -89,14 +89,13 @@ exports.cleanupBuildDir = cleanupBuildDir;
 function formatClosureCompilerError(message) {
   const javaInvocationLine = /Command failed:[^]*--js_output_file=\".*?\"\n/;
   message = message.replace(javaInvocationLine, '');
-  message = highlight(message, {ignoreIllegals: true}); // never throws
+  message = highlight(message, {ignoreIllegals: true});
   message = message.replace(/WARNING/g, colors.yellow('WARNING'));
   message = message.replace(/ERROR/g, colors.red('ERROR'));
   return message;
 }
 
-function compile(entryModuleFilenames, outputDir,
-  outputFilename, options) {
+function compile(entryModuleFilenames, outputDir, outputFilename, options) {
   const hideWarningsFor = [
     'third_party/caja/',
     'third_party/closure-library/sha384-generated.js',
@@ -114,10 +113,8 @@ function compile(entryModuleFilenames, outputDir,
     // Generated code.
     'extensions/amp-access/0.1/access-expr-impl.js',
   ];
-
   const baseExterns = [
     'build-system/amp.extern.js',
-    'third_party/closure-compiler/externs/performance_observer.js',
     'third_party/closure-compiler/externs/web_animations.js',
     'third_party/moment/moment.extern.js',
     'third_party/react-externs/externs.js',
@@ -142,9 +139,11 @@ function compile(entryModuleFilenames, outputDir,
       define.push('ESM_BUILD=true');
     }
 
+    console/*OK*/.assert(typeof entryModuleFilenames == 'string');
+    const entryModule = entryModuleFilenames;
     // TODO(@cramforce): Run the post processing step
     return singlePassCompile(
-        entryModuleFilenames,
+        entryModule,
         compilationOptions
     ).then(() => {
       return new Promise((resolve, reject) => {
@@ -252,7 +251,7 @@ function compile(entryModuleFilenames, outputDir,
       'third_party/webcomponentsjs/ShadowCSS.js',
       'third_party/rrule/rrule.js',
       'third_party/react-dates/bundle.js',
-      'node_modules/dompurify/dist/purify.cjs.js',
+      'node_modules/dompurify/dist/purify.es.js',
       'node_modules/promise-pjs/promise.js',
       'node_modules/set-dom/src/**/*.js',
       'node_modules/web-animations-js/web-animations.install.js',
@@ -352,12 +351,14 @@ function compile(entryModuleFilenames, outputDir,
         rewrite_polyfills: false,
         externs,
         js_module_root: [
-          'node_modules/',
+          // Do _not_ include 'node_modules/' in js_module_root with 'NODE'
+          // resolution or bad things will happen (#18600).
           'build/patched-module/',
           'build/fake-module/',
           'build/fake-polyfills/',
         ],
         entry_point: entryModuleFilenames,
+        module_resolution: 'NODE',
         process_common_js_modules: true,
         // This strips all files from the input set that aren't explicitly
         // required.
@@ -367,12 +368,15 @@ function compile(entryModuleFilenames, outputDir,
         source_map_location_mapping:
             '|' + sourceMapBase,
         warning_level: 'DEFAULT',
+        jscomp_error: [],
+        // moduleLoad: Demote "module not found" errors to ignore missing files
+        //     in type declarations in the swg.js bundle.
+        jscomp_warning: ['moduleLoad'],
         // Turn off warning for "Unknown @define" since we use define to pass
         // args such as FORTESTING to our runner.
         jscomp_off: ['unknownDefines'],
         define,
         hide_warnings_for: hideWarningsFor,
-        jscomp_error: [],
       },
     };
 

diff --git a/build-system/tasks/dep-check.js b/build-system/tasks/dep-check.js
@@ -175,8 +175,13 @@ function getGraph(entryModule) {
 
   // TODO(erwinm): Try and work this in with `gulp build` so that
   // we're not running browserify twice on travis.
-  const bundler = browserify(entryModule, {debug: true, deps: true})
-      .transform(babelify, {compact: false});
+  const bundler = browserify(entryModule, {debug: true})
+      .transform(babelify, {
+        compact: false,
+        // Transform files in node_modules since deps use ES6 export.
+        // https://github.com/babel/babelify#why-arent-files-in-node_modules-being-transformed
+        global: true,
+      });
 
   bundler.pipeline.get('deps').push(through.obj(function(row, enc, next) {
     module.deps.push({

diff --git a/build-system/tasks/karma.conf.js b/build-system/tasks/karma.conf.js
@@ -63,7 +63,15 @@ module.exports = {
     debug: true,
     basedir: __dirname + '/../../',
     transform: [
-      ['babelify', {compact: false, sourceMapsAbsolute: true}],
+      [
+        'babelify', {
+          compact: false,
+          // Transform files in node_modules since deps use ES6 export.
+          // https://github.com/babel/babelify#why-arent-files-in-node_modules-being-transformed
+          global: true,
+          sourceMapsAbsolute: true,
+        },
+      ],
     ],
     // Prevent "cannot find module" errors on Travis. See #14166.
     bundleDelay: process.env.TRAVIS ? 5000 : 1200,

diff --git a/build-system/tasks/runtime-test.js b/build-system/tasks/runtime-test.js
@@ -501,6 +501,9 @@ function runTests() {
     c.browserify.transform = [
       ['babelify', {
         compact: false,
+        // Transform files in node_modules since deps use ES6 export.
+        // https://github.com/babel/babelify#why-arent-files-in-node_modules-being-transformed
+        global: true,
         plugins: [
           ['babel-plugin-istanbul', {
             exclude: [

diff --git a/build-system/tasks/update-packages.js b/build-system/tasks/update-packages.js
@@ -51,7 +51,7 @@ function patchWebAnimations() {
       'node_modules/web-animations-js/' +
       'web-animations.min.js').toString();
   // Wrap the contents inside the install function.
-  file = 'exports.installWebAnimations = function(window) {\n' +
+  file = 'export function installWebAnimations(window) {\n' +
       'var document = window.document;\n' +
       file.replace(/requestAnimationFrame/g, function(a, b) {
         if (file.charAt(b - 1) == '.') {
@@ -80,12 +80,14 @@ function patchRegisterElement() {
   file = fs.readFileSync(
       'node_modules/document-register-element/build/' +
       'document-register-element.node.js').toString();
+
   // Eliminate the immediate side effect.
   if (!/installCustomElements\(global\);/.test(file)) {
     throw new Error('Expected "installCustomElements(global);" ' +
         'to appear in document-register-element');
   }
   file = file.replace('installCustomElements(global);', '');
+
   // Closure Compiler does not generate a `default` property even though
   // to interop CommonJS and ES6 modules. This is the same issue typescript
   // ran into here https://github.com/Microsoft/TypeScript/issues/2719
@@ -94,7 +96,8 @@ function patchRegisterElement() {
         'to appear in document-register-element');
   }
   file = file.replace('module.exports = installCustomElements;',
-      'exports.installCustomElements = installCustomElements;');
+      'export {installCustomElements};');
+
   writeIfUpdated(patchedName, file);
 }
 

diff --git a/extensions/amp-analytics/0.1/iframe-transport.js b/extensions/amp-analytics/0.1/iframe-transport.js
@@ -180,7 +180,7 @@ export class IframeTransport {
               (entry['name'] == 'cross-origin-descendant') &&
               entry.attribution) {
               entry.attribution.forEach(attrib => {
-                if (this.frameUrl_ == attrib.containerSrc &&
+                if (this.frameUrl_ == attrib['containerSrc'] &&
                     ++this.numLongTasks_ % LONG_TASK_REPORTING_THRESHOLD == 0) {
                   user().error(TAG_, `Long Task: Vendor: "${this.type_}"`);
                 }

diff --git a/extensions/amp-date-picker/0.1/amp-date-picker.js b/extensions/amp-date-picker/0.1/amp-date-picker.js
@@ -14,7 +14,6 @@
  * limitations under the License.
  */
 
-import '../../../third_party/react-dates/bundle';
 import {ActionTrust} from '../../../src/action-constants';
 import {AmpEvents} from '../../../src/amp-events';
 import {CSS} from '../../../build/amp-date-picker-0.1.css';

diff --git a/extensions/amp-date-picker/0.1/date-range-picker.js b/extensions/amp-date-picker/0.1/date-range-picker.js
@@ -27,9 +27,14 @@ function createDateRangePickerBase() {
   const {
     DAY_SIZE,
     HORIZONTAL_ORIENTATION,
-  } = requireExternal('react-dates/constants');
-  const {DayPickerRangeController} = requireExternal('react-dates');
-
+  } = /** @type {{DAY_SIZE: number, HORIZONTAL_ORIENTATION: string}} */ (
+    requireExternal('react-dates/constants')
+  );
+  const {
+    DayPickerRangeController,
+  } = /** @type {{DayPickerRangerController: Object}} */ (
+    requireExternal('react-dates')
+  );
 
   const defaultProps = map({
     startDate: null, // TODO: use null

diff --git a/extensions/amp-mustache/0.1/amp-mustache.js b/extensions/amp-mustache/0.1/amp-mustache.js
@@ -80,7 +80,7 @@ export class AmpMustache extends AMP.BaseTemplate {
     container.appendChild(content);
     /** @private @const {string} */
     this.template_ = container./*OK*/innerHTML;
-    mustacheParse(this.template_);
+    mustacheParse(this.template_, /* tags */ undefined);
   }
 
   /** @override */
@@ -91,7 +91,8 @@ export class AmpMustache extends AMP.BaseTemplate {
       if (typeof data === 'object') {
         mustacheData = Object.assign({}, data, this.nestedTemplates_);
       }
-      html = mustacheRender(this.template_, mustacheData);
+      html = mustacheRender(this.template_, mustacheData,
+          /* partials */ undefined);
     }
     return this.serializeHtml_(html);
   }