Filter duplicate extensions by src URL (#1192)

* Filter duplicate extensions by src URL Fixes #1191 * move resource hints up * move runtime styles after resource hints * don't forget the modulepreload... * Sort extensions alphabetically and don't filter nomodule v0.js * workaround for lack of Array#flat in Node10 * swap module and nomodule * inject resource hints before other meta tags * run ReOrderHeadTransformer last * Move viewport after meta charset * clean up
ampproject · Apr 8, 2021 · 359aea3 · 359aea3
1 parent 3ec7bd4
commit 359aea3
Show file tree

Hide file tree

Showing 25 changed files with 180 additions and 130 deletions.
diff --git a/packages/optimizer/lib/DomTransformer.js b/packages/optimizer/lib/DomTransformer.js
@@ -44,16 +44,15 @@ const TRANSFORMATIONS_AMP_FIRST = [
   // Removes the boilerplate
   // needs to run after ServerSideRendering
   'AmpBoilerplateTransformer',
-  // Optimizes script import order
-  // needs to run after ServerSideRendering
-  'ReorderHeadTransformer',
-  // needs to run after ReorderHeadTransformer
   'RewriteAmpUrls',
   'GoogleFontsPreconnect',
   'PruneDuplicateResourceHints',
   'AddBlurryImagePlaceholders',
   // Move keyframes into a separate style tag
   'SeparateKeyframes',
+  // Optimizes script import order
+  // needs to run after ServerSideRendering
+  'ReorderHeadTransformer',
   'AddTransformedFlag',
   // Minifies HTML, JSON, inline amp-script
   'MinifyHtml',
@@ -86,16 +85,15 @@ const TRANSFORMATIONS_PAIRED_AMP = [
   // Removes the boilerplate
   // needs to run after ServerSideRendering
   'AmpBoilerplateTransformer',
-  // Optimizes script import order
-  // needs to run after ServerSideRendering
-  'ReorderHeadTransformer',
-  // needs to run after ReorderHeadTransformer
   'RewriteAmpUrls',
   'GoogleFontsPreconnect',
   'PruneDuplicateResourceHints',
   'AddBlurryImagePlaceholders',
   'SeparateKeyframes',
   'AddTransformedFlag',
+  // Optimizes script import order
+  // needs to run after ServerSideRendering
+  'ReorderHeadTransformer',
   // Minifies HTML, JSON, inline amp-script
   'MinifyHtml',
   // Inject CSP script has required for inline amp-script
@@ -119,13 +117,12 @@ const TRANSFORMATIONS_MINIMAL = [
   // Removes the boilerplate
   // needs to run after ServerSideRendering
   'AmpBoilerplateTransformer',
-  // Optimizes script import order
-  // needs to run after ServerSideRendering
-  'ReorderHeadTransformer',
-  // needs to run after ReorderHeadTransformer
   'RewriteAmpUrls',
   'GoogleFontsPreconnect',
   'PruneDuplicateResourceHints',
+  // Optimizes script import order
+  // needs to run after ServerSideRendering
+  'ReorderHeadTransformer',
   'AddTransformedFlag',
 ];
 

diff --git a/packages/optimizer/lib/transformers/ReorderHeadTransformer.js b/packages/optimizer/lib/transformers/ReorderHeadTransformer.js
@@ -23,11 +23,12 @@ class HeadNodes {
     this._styleAmpRuntime = null;
     this._linkStyleAmpRuntime = null;
     this._metaCharset = null;
-    this._scriptAmpEngine = null;
+    this._metaViewport = null;
+    this._scriptAmpEngine = [];
     this._metaOther = [];
-    this._scriptRenderDelayingExtensions = [];
-    this._scriptNonRenderDelayingExtensions = [];
     this._resourceHintLinks = [];
+    this._scriptRenderDelayingExtensions = new Map();
+    this._scriptNonRenderDelayingExtensions = new Map();
     this._linkIcons = [];
     this._styleAmpCustom = null;
     this._linkStylesheetsBeforeAmpCustom = [];
@@ -41,38 +42,36 @@ class HeadNodes {
   }
 
   uniquifyAndSortCustomElements() {
-    this._scriptRenderDelayingExtensions = this._removeDuplicateCustomExtensions(
+    this._scriptRenderDelayingExtensions = this._sortExtensions(
       this._scriptRenderDelayingExtensions
     );
-    this._scriptNonRenderDelayingExtensions = this._removeDuplicateCustomExtensions(
+    this._scriptNonRenderDelayingExtensions = this._sortExtensions(
       this._scriptNonRenderDelayingExtensions
     );
   }
 
-  _removeDuplicateCustomExtensions(extensions) {
-    const nodesByName = new Map();
-    for (const node of extensions) {
-      const name = this._getName(node);
-      nodesByName.set(name, node);
-    }
-    return Array.from(nodesByName.values());
+  _sortExtensions(extensions) {
+    const sortedExtensions = new Map([...extensions].sort((a, b) => a[0].localeCompare(b[0])));
+    // TODO replace with Array#flat once Node 10 is EOL
+    return [].concat.apply([], Array.from(sortedExtensions.values()));
   }
 
   appendToHead(head) {
     appendChild(head, this._metaCharset);
+    appendChild(head, this._metaViewport);
+    appendAll(head, this._resourceHintLinks);
+    appendAll(head, this._metaOther);
     appendChild(head, this._linkStyleAmpRuntime);
     appendChild(head, this._styleAmpRuntime);
-    appendAll(head, this._metaOther);
-    appendChild(head, this._scriptAmpEngine);
+    appendAll(head, this._scriptAmpEngine);
     appendAll(head, this._scriptRenderDelayingExtensions);
     appendAll(head, this._scriptNonRenderDelayingExtensions);
-    appendAll(head, this._linkIcons);
-    appendAll(head, this._resourceHintLinks);
-    appendAll(head, this._linkStylesheetsBeforeAmpCustom);
     appendChild(head, this._styleAmpCustom);
-    appendAll(head, this._others);
     appendChild(head, this._styleAmpBoilerplate);
     appendChild(head, this._noscript);
+    appendAll(head, this._linkIcons);
+    appendAll(head, this._linkStylesheetsBeforeAmpCustom);
+    appendAll(head, this._others);
   }
 
   _registerNode(node) {
@@ -96,35 +95,47 @@ class HeadNodes {
       this._metaCharset = node;
       return;
     }
+    if (node.attribs.name == 'viewport') {
+      this._metaViewport = node;
+      return;
+    }
     this._metaOther.push(node);
   }
 
   _registerScript(node) {
+    const scriptIndex = hasAttribute(node, 'nomodule') ? 1 : 0;
+    const name = this._getName(node);
     // Currently there are two amp engine tags: v0.js and
     // amp4ads-v0.js.  According to validation rules they are the
     // only script tags with a src attribute and do not have
     // attributes custom-element or custom-template. Record the
     // amp engine tag so it can be emitted first among script
     // tags.
-    if (hasAttribute(node, 'src') && !this._getName(node)) {
-      this._scriptAmpEngine = node;
+    if (hasAttribute(node, 'src') && !name) {
+      this._scriptAmpEngine[scriptIndex] = node;
       return;
     }
     if (hasAttribute(node, 'custom-element')) {
       if (isRenderDelayingExtension(node)) {
-        this._scriptRenderDelayingExtensions.push(node);
+        this._registerExtension(this._scriptRenderDelayingExtensions, name, scriptIndex, node);
         return;
       }
-      this._scriptNonRenderDelayingExtensions.push(node);
+      this._registerExtension(this._scriptNonRenderDelayingExtensions, name, scriptIndex, node);
       return;
     }
     if (hasAttribute(node, 'custom-template')) {
-      this._scriptNonRenderDelayingExtensions.push(node);
+      this._registerExtension(this._scriptNonRenderDelayingExtensions, name, scriptIndex, node);
       return;
     }
     this._others.push(node);
   }
 
+  _registerExtension(collection, name, scriptIndex, node) {
+    const values = collection.get(name) || [];
+    values[scriptIndex] = node;
+    collection.set(name, values);
+  }
+
   _registerStyle(node) {
     if (hasAttribute(node, 'amp-runtime')) {
       this._styleAmpRuntime = node;
@@ -160,7 +171,13 @@ class HeadNodes {
       return;
     }
 
-    if (rel === 'preload' || rel === 'prefetch' || rel === 'dns-prefetch' || rel === 'preconnect') {
+    if (
+      rel === 'preload' ||
+      rel === 'prefetch' ||
+      rel === 'dns-prefetch' ||
+      rel === 'preconnect' ||
+      rel == 'modulepreload'
+    ) {
       this._resourceHintLinks.push(node);
       return;
     }

diff --git a/packages/optimizer/spec/end-to-end/body-only/expected_output.default.html b/packages/optimizer/spec/end-to-end/body-only/expected_output.default.html
@@ -1,13 +1,13 @@
 <!doctype html>
 <html amp i-amphtml-layout i-amphtml-no-boilerplate transformed="self;v=1">
 <head>
-  <meta data-auto charset="utf-8"><style amp-runtime i-amphtml-version="123456789000000">/* ampproject.org/rtv v0.css */amp-img[i-amphtml-ssr]:not(.i-amphtml-element):not([layout=container])>*{display: block;}</style>
+  <meta data-auto charset="utf-8">
   <meta data-auto name="viewport" content="width=device-width,minimum-scale=1,initial-scale=1">
-  <link as="script" crossorigin="anonymous" href="https://cdn.ampproject.org/v0.mjs" rel="modulepreload">
-  <script async nomodule src="https://cdn.ampproject.org/v0.js"></script>
+  <link as="script" crossorigin="anonymous" href="https://cdn.ampproject.org/v0.mjs" rel="modulepreload"><style amp-runtime i-amphtml-version="123456789000000">/* ampproject.org/rtv v0.css */amp-img[i-amphtml-ssr]:not(.i-amphtml-element):not([layout=container])>*{display: block;}</style>
   <script data-auto async src="https://cdn.ampproject.org/v0.mjs" type="module" crossorigin="anonymous"></script>
-  <script async nomodule src="https://cdn.ampproject.org/v0/amp-video-0.1.js" custom-element="amp-video"></script>
+  <script async nomodule src="https://cdn.ampproject.org/v0.js"></script>
   <script async src="https://cdn.ampproject.org/v0/amp-video-0.1.mjs" custom-element="amp-video" type="module" crossorigin="anonymous"></script>
+  <script async nomodule src="https://cdn.ampproject.org/v0/amp-video-0.1.js" custom-element="amp-video"></script>
   <link data-auto rel="canonical" href=".">
 </head>
 <body>

diff --git a/packages/optimizer/spec/end-to-end/body-only/expected_output.lts.html b/packages/optimizer/spec/end-to-end/body-only/expected_output.lts.html
@@ -1,13 +1,13 @@
 <!doctype html>
 <html amp i-amphtml-layout i-amphtml-no-boilerplate transformed="self;v=1">
 <head>
-  <meta data-auto charset="utf-8"><style amp-runtime i-amphtml-version="123456789000000">/* ampproject.org/rtv v0.css */amp-img[i-amphtml-ssr]:not(.i-amphtml-element):not([layout=container])>*{display: block;}</style>
+  <meta data-auto charset="utf-8">
   <meta data-auto name="viewport" content="width=device-width,minimum-scale=1,initial-scale=1">
-  <link as="script" crossorigin="anonymous" href="https://cdn.ampproject.org/lts/v0.mjs" rel="modulepreload">
-  <script async nomodule src="https://cdn.ampproject.org/lts/v0.js"></script>
+  <link as="script" crossorigin="anonymous" href="https://cdn.ampproject.org/lts/v0.mjs" rel="modulepreload"><style amp-runtime i-amphtml-version="123456789000000">/* ampproject.org/rtv v0.css */amp-img[i-amphtml-ssr]:not(.i-amphtml-element):not([layout=container])>*{display: block;}</style>
   <script data-auto async src="https://cdn.ampproject.org/lts/v0.mjs" type="module" crossorigin="anonymous"></script>
-  <script async nomodule src="https://cdn.ampproject.org/lts/v0/amp-video-0.1.js" custom-element="amp-video"></script>
+  <script async nomodule src="https://cdn.ampproject.org/lts/v0.js"></script>
   <script async src="https://cdn.ampproject.org/lts/v0/amp-video-0.1.mjs" custom-element="amp-video" type="module" crossorigin="anonymous"></script>
+  <script async nomodule src="https://cdn.ampproject.org/lts/v0/amp-video-0.1.js" custom-element="amp-video"></script>
   <link data-auto rel="canonical" href=".">
 </head>
 <body>

diff --git a/packages/optimizer/spec/end-to-end/body-only/expected_output.minimal.html b/packages/optimizer/spec/end-to-end/body-only/expected_output.minimal.html
@@ -1,13 +1,13 @@
 <!doctype html>
 <html amp i-amphtml-layout i-amphtml-no-boilerplate transformed="self;v=1">
 <head>
-  <meta data-auto charset="utf-8"><style amp-runtime i-amphtml-version="123456789000000">/* ampproject.org/rtv v0.css */amp-img[i-amphtml-ssr]:not(.i-amphtml-element):not([layout=container])>*{display: block;}</style>
+  <meta data-auto charset="utf-8">
   <meta data-auto name="viewport" content="width=device-width,minimum-scale=1,initial-scale=1">
-  <link as="script" crossorigin="anonymous" href="https://cdn.ampproject.org/v0.mjs" rel="modulepreload">
-  <script async nomodule src="https://cdn.ampproject.org/v0.js"></script>
+  <link as="script" crossorigin="anonymous" href="https://cdn.ampproject.org/v0.mjs" rel="modulepreload"><style amp-runtime i-amphtml-version="123456789000000">/* ampproject.org/rtv v0.css */amp-img[i-amphtml-ssr]:not(.i-amphtml-element):not([layout=container])>*{display: block;}</style>
   <script data-auto async src="https://cdn.ampproject.org/v0.mjs" type="module" crossorigin="anonymous"></script>
-  <script async nomodule src="https://cdn.ampproject.org/v0/amp-video-0.1.js" custom-element="amp-video"></script>
+  <script async nomodule src="https://cdn.ampproject.org/v0.js"></script>
   <script async src="https://cdn.ampproject.org/v0/amp-video-0.1.mjs" custom-element="amp-video" type="module" crossorigin="anonymous"></script>
+  <script async nomodule src="https://cdn.ampproject.org/v0/amp-video-0.1.js" custom-element="amp-video"></script>
   <link data-auto rel="canonical" href=".">
 </head>
 <body>

diff --git a/packages/optimizer/spec/end-to-end/hello-world/expected_output.default.html b/packages/optimizer/spec/end-to-end/hello-world/expected_output.default.html
@@ -1,17 +1,17 @@
 <!doctype html>
 <html ⚡ i-amphtml-layout i-amphtml-no-boilerplate transformed="self;v=1">
 <head>
-  <meta charset="utf-8"><style amp-runtime i-amphtml-version="123456789000000">/* ampproject.org/rtv v0.css */amp-img[i-amphtml-ssr]:not(.i-amphtml-element):not([layout=container])>*{display: block;}</style>
+  <meta charset="utf-8">
   <meta name="viewport" content="width=device-width,minimum-scale=1">
-  <link as="script" crossorigin="anonymous" href="https://cdn.ampproject.org/v0.mjs" rel="modulepreload">
-  <script async nomodule src="https://cdn.ampproject.org/v0.js"></script>
+  <link as="script" crossorigin="anonymous" href="https://cdn.ampproject.org/v0.mjs" rel="modulepreload"><style amp-runtime i-amphtml-version="123456789000000">/* ampproject.org/rtv v0.css */amp-img[i-amphtml-ssr]:not(.i-amphtml-element):not([layout=container])>*{display: block;}</style>
   <script async src="https://cdn.ampproject.org/v0.mjs" type="module" crossorigin="anonymous"></script>
-  <script async nomodule src="https://cdn.ampproject.org/v0/amp-experiment-0.1.js" custom-element="amp-experiment"></script>
+  <script async nomodule src="https://cdn.ampproject.org/v0.js"></script>
   <script async custom-element="amp-experiment" src="https://cdn.ampproject.org/v0/amp-experiment-0.1.mjs" type="module" crossorigin="anonymous"></script>
-  <script async nomodule src="https://cdn.ampproject.org/v0/amp-video-0.1.js" custom-element="amp-video"></script>
-  <script async src="https://cdn.ampproject.org/v0/amp-video-0.1.mjs" custom-element="amp-video" type="module" crossorigin="anonymous"></script>
-  <script async nomodule src="https://cdn.ampproject.org/v0/amp-analytics-0.1.js" custom-element="amp-analytics"></script>
+  <script async nomodule src="https://cdn.ampproject.org/v0/amp-experiment-0.1.js" custom-element="amp-experiment"></script>
   <script async custom-element="amp-analytics" src="https://cdn.ampproject.org/v0/amp-analytics-0.1.mjs" type="module" crossorigin="anonymous"></script>
+  <script async nomodule src="https://cdn.ampproject.org/v0/amp-analytics-0.1.js" custom-element="amp-analytics"></script>
+  <script async src="https://cdn.ampproject.org/v0/amp-video-0.1.mjs" custom-element="amp-video" type="module" crossorigin="anonymous"></script>
+  <script async nomodule src="https://cdn.ampproject.org/v0/amp-video-0.1.js" custom-element="amp-video"></script>
   <link rel="canonical" href="self.html"><style amp-custom>h1{margin:16px}</style>
 </head>
 <body>

diff --git a/packages/optimizer/spec/end-to-end/hello-world/expected_output.lts.html b/packages/optimizer/spec/end-to-end/hello-world/expected_output.lts.html
@@ -1,17 +1,17 @@
 <!doctype html>
 <html ⚡ i-amphtml-layout i-amphtml-no-boilerplate transformed="self;v=1">
 <head>
-  <meta charset="utf-8"><style amp-runtime i-amphtml-version="123456789000000">/* ampproject.org/rtv v0.css */amp-img[i-amphtml-ssr]:not(.i-amphtml-element):not([layout=container])>*{display: block;}</style>
+  <meta charset="utf-8">
   <meta name="viewport" content="width=device-width,minimum-scale=1">
-  <link as="script" crossorigin="anonymous" href="https://cdn.ampproject.org/lts/v0.mjs" rel="modulepreload">
-  <script async nomodule src="https://cdn.ampproject.org/lts/v0.js"></script>
+  <link as="script" crossorigin="anonymous" href="https://cdn.ampproject.org/lts/v0.mjs" rel="modulepreload"><style amp-runtime i-amphtml-version="123456789000000">/* ampproject.org/rtv v0.css */amp-img[i-amphtml-ssr]:not(.i-amphtml-element):not([layout=container])>*{display: block;}</style>
   <script async src="https://cdn.ampproject.org/lts/v0.mjs" type="module" crossorigin="anonymous"></script>
-  <script async nomodule src="https://cdn.ampproject.org/lts/v0/amp-experiment-0.1.js" custom-element="amp-experiment"></script>
+  <script async nomodule src="https://cdn.ampproject.org/lts/v0.js"></script>
   <script async custom-element="amp-experiment" src="https://cdn.ampproject.org/lts/v0/amp-experiment-0.1.mjs" type="module" crossorigin="anonymous"></script>
-  <script async nomodule src="https://cdn.ampproject.org/lts/v0/amp-video-0.1.js" custom-element="amp-video"></script>
-  <script async src="https://cdn.ampproject.org/lts/v0/amp-video-0.1.mjs" custom-element="amp-video" type="module" crossorigin="anonymous"></script>
-  <script async nomodule src="https://cdn.ampproject.org/lts/v0/amp-analytics-0.1.js" custom-element="amp-analytics"></script>
+  <script async nomodule src="https://cdn.ampproject.org/lts/v0/amp-experiment-0.1.js" custom-element="amp-experiment"></script>
   <script async custom-element="amp-analytics" src="https://cdn.ampproject.org/lts/v0/amp-analytics-0.1.mjs" type="module" crossorigin="anonymous"></script>
+  <script async nomodule src="https://cdn.ampproject.org/lts/v0/amp-analytics-0.1.js" custom-element="amp-analytics"></script>
+  <script async src="https://cdn.ampproject.org/lts/v0/amp-video-0.1.mjs" custom-element="amp-video" type="module" crossorigin="anonymous"></script>
+  <script async nomodule src="https://cdn.ampproject.org/lts/v0/amp-video-0.1.js" custom-element="amp-video"></script>
   <link rel="canonical" href="self.html"><style amp-custom>h1{margin:16px}</style>
 </head>
 <body>

diff --git a/packages/optimizer/spec/end-to-end/hello-world/expected_output.minimal.html b/packages/optimizer/spec/end-to-end/hello-world/expected_output.minimal.html
@@ -1,17 +1,17 @@
 <!doctype html>
 <html ⚡ i-amphtml-layout i-amphtml-no-boilerplate transformed="self;v=1">
 <head>
-  <meta charset="utf-8"><style amp-runtime i-amphtml-version="123456789000000">/* ampproject.org/rtv v0.css */amp-img[i-amphtml-ssr]:not(.i-amphtml-element):not([layout=container])>*{display: block;}</style>
+  <meta charset="utf-8">
   <meta name="viewport" content="width=device-width,minimum-scale=1">
-  <link as="script" crossorigin="anonymous" href="https://cdn.ampproject.org/v0.mjs" rel="modulepreload">
-  <script async nomodule src="https://cdn.ampproject.org/v0.js"></script>
+  <link as="script" crossorigin="anonymous" href="https://cdn.ampproject.org/v0.mjs" rel="modulepreload"><style amp-runtime i-amphtml-version="123456789000000">/* ampproject.org/rtv v0.css */amp-img[i-amphtml-ssr]:not(.i-amphtml-element):not([layout=container])>*{display: block;}</style>
   <script async src="https://cdn.ampproject.org/v0.mjs" type="module" crossorigin="anonymous"></script>
-  <script async nomodule src="https://cdn.ampproject.org/v0/amp-experiment-0.1.js" custom-element="amp-experiment"></script>
+  <script async nomodule src="https://cdn.ampproject.org/v0.js"></script>
   <script async custom-element="amp-experiment" src="https://cdn.ampproject.org/v0/amp-experiment-0.1.mjs" type="module" crossorigin="anonymous"></script>
-  <script async nomodule src="https://cdn.ampproject.org/v0/amp-video-0.1.js" custom-element="amp-video"></script>
-  <script async src="https://cdn.ampproject.org/v0/amp-video-0.1.mjs" custom-element="amp-video" type="module" crossorigin="anonymous"></script>
-  <script async nomodule src="https://cdn.ampproject.org/v0/amp-analytics-0.1.js" custom-element="amp-analytics"></script>
+  <script async nomodule src="https://cdn.ampproject.org/v0/amp-experiment-0.1.js" custom-element="amp-experiment"></script>
   <script async custom-element="amp-analytics" src="https://cdn.ampproject.org/v0/amp-analytics-0.1.mjs" type="module" crossorigin="anonymous"></script>
+  <script async nomodule src="https://cdn.ampproject.org/v0/amp-analytics-0.1.js" custom-element="amp-analytics"></script>
+  <script async src="https://cdn.ampproject.org/v0/amp-video-0.1.mjs" custom-element="amp-video" type="module" crossorigin="anonymous"></script>
+  <script async nomodule src="https://cdn.ampproject.org/v0/amp-video-0.1.js" custom-element="amp-video"></script>
   <link rel="canonical" href="self.html"><style amp-custom>h1 { margin: 16px; }</style>
 </head>
 <body>