Repository: ProjectEvergreen/greenwood (js)

Issue:
greenwood not bundling non module script tags (e.g. <script `type="module" ...>`)
## Type of Change
- [ ] New Feature Request
- [ ] Documentation / Website
- [ ] Improvement / Suggestion
- [x] Bug
- [ ] Other (please clarify below)

## Summary
It seems that if you have just a `<script>` like _without_ a `type="module"` attribute, it will not get bundled
```html
<script src="/some/path/components/link-target.js"></script>
```

then it will have a 404 in your `build` output
![Screen Shot 2021-08-24 at 9 11 24 PM](https://user-images.githubusercontent.com/895923/130710215-70209a11-fe47-4ff5-b687-283ddb9df538.png)

> See https://github.com/thescientist13/greenwood-starter-presentation/pull/38 for reference

## Details

For some reason i made the assumption / choice to only support module script tags?  Not sure what my thinking of was for that, but I don't think it should need to be this restrictive?  🤔 

Should make sure to double check this is covered by our test cases then.

Pull request:
allow for non module script tag bundling
<!--
## Submitting a Pull Request
We love contributions and appreciate any help you can offer!
-->

## Related Issue
resolves #702 

## Summary of Changes
1.  Allow for non `type="module"` script tags to get bundled

## TODOs
1. [x] Validate via a test case

Patch:
diff --git a/packages/cli/src/config/rollup.config.js b/packages/cli/src/config/rollup.config.js
index 604f4a71..4d761114 100644
--- a/packages/cli/src/config/rollup.config.js
+++ b/packages/cli/src/config/rollup.config.js
@@ -170,9 +170,8 @@ function greenwoodHtmlPlugin(compilation) {
 
           headScripts.forEach((scriptTag) => {
             const parsedAttributes = parseTagForAttributes(scriptTag);
-     
-            // handle <script type="module" src="some/path.js"></script>
-            if (!isRemoteUrl(parsedAttributes.src) && parsedAttributes.type === 'module' && parsedAttributes.src && !mappedScripts.get(parsedAttributes.src)) {
+            // handle <script [type="module"] src="some/path.js"></script>
+            if (!isRemoteUrl(parsedAttributes.src) && parsedAttributes.src && !mappedScripts.get(parsedAttributes.src)) {
               if (optimization === 'static' || parsedAttributes['data-gwd-opt'] === 'static') {
                 // dont need to bundle / emit this one
               } else {
@@ -309,8 +308,8 @@ function greenwoodHtmlPlugin(compilation) {
             headScripts.forEach((scriptTag) => {
               const parsedAttributes = parseTagForAttributes(scriptTag);
     
-              // handle <script type="module" src="some/path.js"></script>
-              if (!isRemoteUrl(parsedAttributes.src) && parsedAttributes.type === 'module' && parsedAttributes.src) {
+              // handle <script [type="module"] src="some/path.js"></script>
+              if (!isRemoteUrl(parsedAttributes.src) && parsedAttributes.src) {
                 for (const innerBundleId of Object.keys(bundles)) {
                   const { src } = parsedAttributes;
                   const facadeModuleId = bundles[innerBundleId].facadeModuleId
diff --git a/packages/plugin-polyfills/src/index.js b/packages/plugin-polyfills/src/index.js
index 167faeb9..4c59d8e4 100644
--- a/packages/plugin-polyfills/src/index.js
+++ b/packages/plugin-polyfills/src/index.js
@@ -41,7 +41,7 @@ class PolyfillsResource extends ResourceInterface {
 
         const newHtml = body.replace('<head>', `
           <head>
-            <script src="/${filename}"></script>
+            <script src="/${nodeModuleRoot}/${filename}"></script>
         `);
 
         resolve(newHtml);


Test patch:
diff --git a/packages/cli/test/cases/build.default.workspace-javascript-css/build.default.workspace-javascript-css.spec.js b/packages/cli/test/cases/build.default.workspace-javascript-css/build.default.workspace-javascript-css.spec.js
index 88d0a80c..62083abb 100644
--- a/packages/cli/test/cases/build.default.workspace-javascript-css/build.default.workspace-javascript-css.spec.js
+++ b/packages/cli/test/cases/build.default.workspace-javascript-css/build.default.workspace-javascript-css.spec.js
@@ -51,9 +51,9 @@ describe('Build Greenwood With: ', function() {
       dom = await JSDOM.fromFile(path.resolve(this.context.publicDir, 'index.html'));
     });
 
-    describe('<script type="module" src="..."></script> tag in the <head>', function() {
+    describe('<script type="module" src="..."></script> tags in the <head>', function() {
       it('should have one <script> tag for main.js loaded in the <head>', function() {
-        const scriptTags = dom.window.document.querySelectorAll('head > script[src]');
+        const scriptTags = dom.window.document.querySelectorAll('head > script[type="module"]');
         const mainScriptTags = Array.prototype.slice.call(scriptTags).filter(script => {
           return (/main.*.js/).test(script.src);
         });
@@ -62,7 +62,7 @@ describe('Build Greenwood With: ', function() {
       });
 
       it('should have one <script> tag for other.js loaded in the <head>', function() {
-        const scriptTags = dom.window.document.querySelectorAll('head > script[src]');
+        const scriptTags = dom.window.document.querySelectorAll('head > script[type="module"]');
         const mainScriptTags = Array.prototype.slice.call(scriptTags).filter(script => {
           return (/other.*.js/).test(script.src);
         });
@@ -70,8 +70,9 @@ describe('Build Greenwood With: ', function() {
         expect(mainScriptTags.length).to.be.equal(1);
       });
 
+      // 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, '*.js'))).to.have.lengthOf(2);
+        expect(await glob.promise(path.join(this.context.publicDir, '*.js'))).to.have.lengthOf(3);
       });
 
       it('should have the expected main.js file in the output directory', async function() {
@@ -89,7 +90,7 @@ describe('Build Greenwood With: ', function() {
       });
     });
 
-    describe('<script>...</script> tag in the <head> with mixed attribute ordering', function() {
+    describe('<script ...>...</script> tag in the <head> with mixed attribute ordering', function() {
       it('should have two <script> tag with inline script in the <head>', function() {
         const scriptTagInline = dom.window.document.querySelectorAll('head > script:not([src])');
         
@@ -128,6 +129,27 @@ describe('Build Greenwood With: ', function() {
 
     });
 
+    describe('non module <script src="..."></script> tag in the <head>', function() {
+      it('should have one <script> tag for non-module.js loaded in the <head>', function() {
+        const scriptTags = dom.window.document.querySelectorAll('head > script[src]');
+        const mainScriptTags = Array.prototype.slice.call(scriptTags).filter(script => {
+          return (/non-module.*.js/).test(script.src);
+        });
+        
+        expect(mainScriptTags.length).to.be.equal(1);
+      });
+
+      it('should have the expected non-module.js file in the output directory', async function() {
+        expect(await glob.promise(path.join(this.context.publicDir, 'non-module.*.js'))).to.have.lengthOf(1);
+      });
+
+      it('should have the expected output from non-module.js file in index.html', async function() {
+        const scriptTagSrc = dom.window.document.querySelector('body > .output-script-non-module');
+
+        expect(scriptTagSrc.textContent).to.be.equal('script tag non module with src');
+      });
+    });
+
     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');
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
index f51ad3f9..fcfa869c 100644
--- 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
@@ -46,11 +46,14 @@
           document.getElementsByClassName('output-json-fetch')[0].innerHTML = msg;
         })
     </script>
+
+    <script src="../scripts/non-module.js"></script>
   </head>
 
   <body>
     <p class="output-script-src"></p>
     <p class="output-script-inline-one"></p>
+    <p class="output-script-non-module"></p>
     <p class="output-style"></p>
     <p class="output-link"></p>
     <p class="module-shim">let me tell you about type="module-shim"</p>
diff --git a/packages/cli/test/cases/build.default.workspace-javascript-css/src/scripts/non-module.js b/packages/cli/test/cases/build.default.workspace-javascript-css/src/scripts/non-module.js
new file mode 100644
index 00000000..acda2df7
--- /dev/null
+++ b/packages/cli/test/cases/build.default.workspace-javascript-css/src/scripts/non-module.js
@@ -0,0 +1,3 @@
+window.addEventListener('DOMContentLoaded', () => {
+  document.querySelector('p.output-script-non-module').innerHTML = 'script tag non module with src';
+});
\ No newline at end of file