properly bundle all <script> tags (#995)

* propery bundle all script tags * test file corrections
ProjectEvergreen · Nov 3, 2022 · c002185 · c002185
1 parent 4e650cd
commit c002185
Show file tree

Hide file tree

Showing 6 changed files with 107 additions and 75 deletions.
diff --git a/packages/cli/src/lifecycles/prerender.js b/packages/cli/src/lifecycles/prerender.js
@@ -6,7 +6,7 @@ import path from 'path';
 import { WorkerPool } from '../lib/threadpool.js';
 
 function isLocalLink(url = '') {
-  return url.indexOf('http') !== 0 && url.indexOf('//') !== 0;
+  return url !== '' && (url.indexOf('http') !== 0 && url.indexOf('//') !== 0);
 }
 
 function createOutputDirectory(route, outputPathDir) {
@@ -29,7 +29,8 @@ function trackResourcesForRoute(html, compilation, route) {
     style: true
   });
 
-  const scripts = root.querySelectorAll('head script')
+  // intentionally support <script> tags from the <head> or <body>
+  const scripts = root.querySelectorAll('script')
     .filter(script => (
       isLocalLink(script.getAttribute('src')) || script.rawText)
       && script.rawAttrs.indexOf('importmap') < 0)

diff --git a/packages/cli/src/plugins/resource/plugin-standard-html.js b/packages/cli/src/plugins/resource/plugin-standard-html.js
@@ -410,82 +410,74 @@ class StandardHtmlResource extends ResourceInterface {
 
     return new Promise((resolve, reject) => {
       try {
-        const hasHead = body.match(/\<head>(.*)<\/head>/s);
-
-        if (hasHead && hasHead.length > 0) {
-          let headContents = hasHead[0];
-
-          for (const pageResource of pageResources) {
-            const keyedResource = this.compilation.resources.get(pageResource.sourcePathURL.pathname);
-            const { contents, src, type, optimizationAttr, optimizedFileContents, optimizedFileName, rawAttributes } = keyedResource;
-
-            if (src) {
-              if (type === 'script') {
-                if (!optimizationAttr && optimization === 'default') {
-                  const optimizedFilePath = `/${optimizedFileName}`;
-
-                  headContents = headContents.replace(src, optimizedFilePath);
-                  headContents = headContents.replace('<head>', `
-                    <head>
-                    <link rel="modulepreload" href="${optimizedFilePath}" as="script">
-                  `);
-                } else if (optimizationAttr === 'inline' || optimization === 'inline') {
-                  const isModule = rawAttributes.indexOf('type="module') >= 0 ? ' type="module"' : '';
-
-                  headContents = headContents.replace(`<script ${rawAttributes}></script>`, `
-                    <script ${isModule}>
-                      ${optimizedFileContents.replace(/\.\//g, '/').replace(/\$/g, '$$$')}
-                    </script>
-                  `);
-                } else if (optimizationAttr === 'static' || optimization === 'static') {
-                  headContents = headContents.replace(`<script ${rawAttributes}></script>`, '');
-                }
-              } else if (type === 'link') {
-                if (!optimizationAttr && (optimization !== 'none' && optimization !== 'inline')) {
-                  const optimizedFilePath = `/${optimizedFileName}`;
-
-                  headContents = headContents.replace(src, optimizedFilePath);
-                  headContents = headContents.replace('<head>', `
-                    <head>
-                    <link rel="preload" href="${optimizedFilePath}" as="style" crossorigin="anonymous"></link>
-                  `);
-                } else if (optimizationAttr === 'inline' || optimization === 'inline') {
-                  // https://github.com/ProjectEvergreen/greenwood/issues/810
-                  // when pre-rendering, puppeteer normalizes everything to <link .../>
-                  // but if not using pre-rendering, then it could come out as <link ...></link>
-                  // not great, but best we can do for now until #742
-                  headContents = headContents.replace(`<link ${rawAttributes}>`, `
-                    <style>
-                      ${optimizedFileContents}
-                    </style>
-                  `).replace(`<link ${rawAttributes}/>`, `
-                    <style>
-                      ${optimizedFileContents}
-                    </style>
-                  `);
-                }
+        for (const pageResource of pageResources) {
+          const keyedResource = this.compilation.resources.get(pageResource.sourcePathURL.pathname);
+          const { contents, src, type, optimizationAttr, optimizedFileContents, optimizedFileName, rawAttributes } = keyedResource;
+
+          if (src) {
+            if (type === 'script') {
+              if (!optimizationAttr && optimization === 'default') {
+                const optimizedFilePath = `/${optimizedFileName}`;
+
+                body = body.replace(src, optimizedFilePath);
+                body = body.replace('<head>', `
+                  <head>
+                  <link rel="modulepreload" href="${optimizedFilePath}" as="script">
+                `);
+              } else if (optimizationAttr === 'inline' || optimization === 'inline') {
+                const isModule = rawAttributes.indexOf('type="module') >= 0 ? ' type="module"' : '';
+
+                body = body.replace(`<script ${rawAttributes}></script>`, `
+                  <script ${isModule}>
+                    ${optimizedFileContents.replace(/\.\//g, '/').replace(/\$/g, '$$$')}
+                  </script>
+                `);
+              } else if (optimizationAttr === 'static' || optimization === 'static') {
+                body = body.replace(`<script ${rawAttributes}></script>`, '');
               }
-            } else {
-              if (type === 'script') {
-                if (optimizationAttr === 'static' || optimization === 'static') {
-                  headContents = headContents.replace(`<script ${rawAttributes}>${contents.replace(/\.\//g, '/').replace(/\$/g, '$$$')}</script>`, '');
-                } else if (optimizationAttr === 'none') {
-                  headContents = headContents.replace(contents, contents.replace(/\.\//g, '/').replace(/\$/g, '$$$'));
-                } else {
-                  headContents = headContents.replace(contents, optimizedFileContents.replace(/\.\//g, '/').replace(/\$/g, '$$$'));
-                }
-              } else if (type === 'style') {
-                headContents = headContents.replace(contents, optimizedFileContents);
+            } else if (type === 'link') {
+              if (!optimizationAttr && (optimization !== 'none' && optimization !== 'inline')) {
+                const optimizedFilePath = `/${optimizedFileName}`;
+
+                body = body.replace(src, optimizedFilePath);
+                body = body.replace('<head>', `
+                  <head>
+                  <link rel="preload" href="${optimizedFilePath}" as="style" crossorigin="anonymous"></link>
+                `);
+              } else if (optimizationAttr === 'inline' || optimization === 'inline') {
+                // https://github.com/ProjectEvergreen/greenwood/issues/810
+                // when pre-rendering, puppeteer normalizes everything to <link .../>
+                // but if not using pre-rendering, then it could come out as <link ...></link>
+                // not great, but best we can do for now until #742
+                body = body.replace(`<link ${rawAttributes}>`, `
+                  <style>
+                    ${optimizedFileContents}
+                  </style>
+                `).replace(`<link ${rawAttributes}/>`, `
+                  <style>
+                    ${optimizedFileContents}
+                  </style>
+                `);
+              }
+            }
+          } else {
+            if (type === 'script') {
+              if (optimizationAttr === 'static' || optimization === 'static') {
+                body = body.replace(`<script ${rawAttributes}>${contents.replace(/\.\//g, '/').replace(/\$/g, '$$$')}</script>`, '');
+              } else if (optimizationAttr === 'none') {
+                body = body.replace(contents, contents.replace(/\.\//g, '/').replace(/\$/g, '$$$'));
+              } else {
+                body = body.replace(contents, optimizedFileContents.replace(/\.\//g, '/').replace(/\$/g, '$$$'));
               }
+            } else if (type === 'style') {
+              body = body.replace(contents, optimizedFileContents);
             }
           }
-
-          // TODO clean up lit-polyfill as part of https://github.com/ProjectEvergreen/greenwood/issues/728
-          headContents = headContents.replace(/<script src="(.*lit\/polyfill-support.js)"><\/script>/, '');
-
-          body = body.replace(/\<head>(.*)<\/head>/s, headContents.replace(/\$/g, '$$$')); // https://github.com/ProjectEvergreen/greenwood/issues/656);
         }
 
+        // TODO clean up lit-polyfill as part of https://github.com/ProjectEvergreen/greenwood/issues/728
+        body = body.replace(/<script src="(.*lit\/polyfill-support.js)"><\/script>/, '');
+
         resolve(body);
       } catch (e) {
         reject(e);

diff --git a/...ses/build.default.workspace-javascript-css/build.default.workspace-javascript-css.spec.js b/...ses/build.default.workspace-javascript-css/build.default.workspace-javascript-css.spec.js
@@ -66,7 +66,7 @@ describe('Build Greenwood With: ', function() {
 
       // this includes the non module file in a spec below
       it('should have the expected number of bundled .js files in the output directory', async function() {
-        expect(await glob.promise(path.join(this.context.publicDir, '*.*[a-z0-9].js'))).to.have.lengthOf(2);
+        expect(await glob.promise(path.join(this.context.publicDir, '*.*[a-z0-9].js'))).to.have.lengthOf(3);
       });
 
       it('should have the expected other.js file in the output directory', async function() {
@@ -118,6 +118,23 @@ describe('Build Greenwood With: ', function() {
       });
     });
 
+    describe('popup <script src="..."></script> tag in the <body>', function() {
+      it('should have one <script> tag for popup.js loaded in the <head>', function() {
+        const scriptTags = dom.window.document.querySelectorAll('body > script[src]');
+        const popupScriptTags = Array.prototype.slice.call(scriptTags).filter(script => {
+          const src = script.getAttribute('src');
+
+          return (/popup.*[a-z0-9].js/).test(src) && src.indexOf('//') < 0;
+        });
+
+        expect(popupScriptTags.length).to.be.equal(1);
+      });
+
+      it('should have the expected popup.js file in the output directory', async function() {
+        expect(await glob.promise(path.join(this.context.publicDir, 'popup.*[a-z0-9].js'))).to.have.lengthOf(1);
+      });
+    });
+
     describe('<style>...</style> tag in the <head>', function() {
       it('should have one <style> tag in the <head>', function() {
         const styleTags = dom.window.document.querySelectorAll('head > style');
@@ -134,7 +151,7 @@ describe('Build Greenwood With: ', function() {
       it('should have the expected output from the second inline <style> tag in index.html', async function() {
         const styleTags = dom.window.document.querySelectorAll('head > style');
 
-        expect(styleTags[1].textContent.replace(/\n/g, '').trim().replace(' ', '')).to.contain('span.output-style{color:red}');
+        expect(styleTags[1].textContent.replace(/\n/g, '').replace(/ /g, '')).to.contain('span.output-style{color:red}');
       });
 
       it('should have the color style for the output element', function() {
@@ -145,6 +162,20 @@ describe('Build Greenwood With: ', function() {
       });
     });
 
+    describe('<style>...</style> tag in the <body>', function() {
+      it('should have one <style> tag in the <body>', function() {
+        const styleTags = dom.window.document.querySelectorAll('body > style');
+
+        expect(styleTags.length).to.be.equal(1);
+      });
+
+      it('should have the expected output from the first inline <style> tag in index.html in the <body>', async function() {
+        const styleTags = dom.window.document.querySelectorAll('body > style');
+
+        expect(styleTags[0].textContent.replace(/\n/g, '').replace(/ /g, '')).to.contain('h1.popup{color:red;}');
+      });
+    });
+
     describe('<link rel="stylesheet" href="..."/> tag in the <head> with mixed attribute ordering', function() {
       it('should have two <link> tag in the <head>', function() {
         const linkTags = dom.window.document.querySelectorAll('head > link[rel="stylesheet"]');

diff --git a/packages/cli/test/cases/build.default.workspace-javascript-css/src/pages/index.html b/packages/cli/test/cases/build.default.workspace-javascript-css/src/pages/index.html
@@ -60,6 +60,13 @@
     <p class="output-json-fetch"></p>
 
     <span>CSS output two</span>
+
+    <script src="/scripts/popup.js"></script>
+    <style>
+      h1.popup {
+        color: red;
+      }
+    </style>
   </body>
 
 </html>
diff --git a/packages/cli/test/cases/build.default.workspace-javascript-css/src/scripts/popup.js b/packages/cli/test/cases/build.default.workspace-javascript-css/src/scripts/popup.js
@@ -0,0 +1 @@
+alert('surprise!');
diff --git a/packages/plugin-polyfills/test/cases/dsd/dsd.spec.js b/packages/plugin-polyfills/test/cases/dsd/dsd.spec.js
@@ -74,7 +74,7 @@ describe('Build Greenwood With: ', function() {
       it('should have the expected DSD polyfill content in the polyfill <script> tag', function() {
         const scriptTags = dom.window.document.querySelectorAll('body > script');
 
-        expect(scriptTags[0].textContent).to.contain('(function attachShadowRoots(root) {');
+        expect(scriptTags[0].textContent).to.contain('HTMLTemplateElement.prototype.hasOwnProperty("shadowRoot")||function t(o){o.querySelectorAll("template[shadowroot]")');
       });
     });
   });