Repository: ProjectEvergreen/greenwood (js)

Issue:
inline optimization string replace calls that have a lookahead Regex inside a template literal (`$``) in the replacement string breaks users pages
## Type of Change
- [ ] New Feature Request
- [ ] Documentation / Website
- [ ] Improvement / Suggestion
- [x] Bug
- [ ] Other (please clarify below)

## Summary
Had been [working through an issue I saw](https://github.com/ProjectEvergreen/greenwood/pull/495) while testing our optimizations options and found a conflict between MPA mode and inlining optimization.

Basically the inlining was completely breaking the router, seemingly on this Regex inside [the shelf component](https://github.com/ProjectEvergreen/greenwood/blob/v0.13.0/www/components/shelf/shelf.js#L41)
![image](https://user-images.githubusercontent.com/895923/124807778-26f3d080-df2c-11eb-8428-ab14cf24d19e.png)

![image](https://user-images.githubusercontent.com/895923/124807800-2d824800-df2c-11eb-825b-1d86ccb23168.png)

With this being the area of code in question.
```js
 expandRoute(path) {
    let routeShelfListIndex = this.shelfList.findIndex(item => {
      let expRoute = new RegExp(`^${path}$`);
      return expRoute.test(item.route);
    });
```

## Details
After some digging, I found what [the issue was](https://github.com/ProjectEvergreen/greenwood/pull/495#issuecomment-875810894).  ([MDN](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replace#specifying_a_string_as_a_parameter) saves the day!)

>  $` | Inserts the portion of the string that precedes the matched substring.


Essentially, the Regex inside the source code is getting interpreted by `replace` as a parameter it should do something with.

So I guess would need to go through and check all usages of `replace` to make sure we're handling source code appropriately or do some kind of escaping.  Seems like this would need to be done in a lot of places... 😬 
https://stackoverflow.com/a/22612228/417806

Pull request:
handle (inadvertent) lookahead patterns when using `replace` with inline JavaScript
<!--
## Submitting a Pull Request
We love contributions and appreciate any help you can offer!
-->

## Related Issue
resolves #656 

## Summary of Changes
1. Added repro with (failing) test case (the bug [is here](https://github.com/ProjectEvergreen/greenwood/blob/v0.13.0/packages/cli/src/config/rollup.config.js#L432) specifically)
1. Enabled 'inline` mode for the website, [looks promising](https://github.com/ProjectEvergreen/greenwood/pull/657#issuecomment-879201657)

> _Note: given the need to audit replace usage, this will likely not land in the upcoming [`v0.14.0`](https://github.com/ProjectEvergreen/greenwood/issues?q=label%3Av0.14.0+) release as there is a large queue of PRs already lined up_

## TODOs
1. [x] Fix failing test case and validate with [Greenwood website test](https://github.com/ProjectEvergreen/greenwood/pull/495/files)
1. [x] Audit and address other potential "leaks" in using `replace` in the project 

Patch:
diff --git a/greenwood.config.js b/greenwood.config.js
index 53ea7382..4b4a24f4 100644
--- a/greenwood.config.js
+++ b/greenwood.config.js
@@ -13,6 +13,7 @@ const FAVICON_HREF = '/assets/favicon.ico';
 module.exports = {
   workspace: path.join(__dirname, 'www'),
   mode: 'mpa',
+  optimization: 'inline',
   title: 'Greenwood',
   meta: [
     { name: 'description', content: META_DESCRIPTION },
diff --git a/packages/cli/src/config/rollup.config.js b/packages/cli/src/config/rollup.config.js
index 969b83a6..04861ff8 100644
--- a/packages/cli/src/config/rollup.config.js
+++ b/packages/cli/src/config/rollup.config.js
@@ -428,7 +428,9 @@ function greenwoodHtmlPlugin(compilation) {
                 ? process.cwd()
                 : outputDir;
               const outputPath = path.join(basePath, src);
-              const js = fs.readFileSync(outputPath, 'utf-8');
+              const js = fs.readFileSync(outputPath, 'utf-8')
+                .replace(/\$/g, '$$$') // https://github.com/ProjectEvergreen/greenwood/issues/656
+                .replace(/\.\//g, '/'); // force absolute paths
               scratchFiles[src] = true;
 
               html = html.replace(`<script ${scriptTag.rawAttrs}></script>`, `
@@ -447,6 +449,7 @@ function greenwoodHtmlPlugin(compilation) {
               for (const innerBundleId of Object.keys(bundles)) {
                 if (innerBundleId.indexOf(`-${tokenSuffix}`) > 0 && path.extname(innerBundleId) === '.js') {
                   const bundledSource = fs.readFileSync(path.join(outputDir, innerBundleId), 'utf-8')
+                    .replace(/\$/g, '$$$') // https://github.com/ProjectEvergreen/greenwood/issues/656
                     .replace(/\.\//g, '/'); // force absolute paths
                   
                   if (markerRegex.test(bundledSource)) {
@@ -485,6 +488,7 @@ function greenwoodHtmlPlugin(compilation) {
                 `);
               }
             });
+
           // mark each HTML's sourcemap file (generated by Rollup) to be cleaned up
           // would be nice if we could just prevent Rollup from generating sourcemaps for just our input files in the first place (GFI)
           // https://github.com/ProjectEvergreen/greenwood/issues/659
diff --git a/packages/cli/src/plugins/resource/plugin-node-modules.js b/packages/cli/src/plugins/resource/plugin-node-modules.js
index 360b1cd8..117f7352 100644
--- a/packages/cli/src/plugins/resource/plugin-node-modules.js
+++ b/packages/cli/src/plugins/resource/plugin-node-modules.js
@@ -243,7 +243,7 @@ class NodeModulesResource extends ResourceInterface {
           const contents = hasHead[0]
             .replace(/type="module"/g, 'type="module-shim"');
 
-          newContents = newContents.replace(/\<head>(.*)<\/head>/s, contents);
+          newContents = newContents.replace(/\<head>(.*)<\/head>/s, contents.replace(/\$/g, '$$$')); // https://github.com/ProjectEvergreen/greenwood/issues/656);
         }
 
         const userPackageJson = fs.existsSync(`${userWorkspace}/package.json`)
diff --git a/packages/cli/src/plugins/resource/plugin-standard-html.js b/packages/cli/src/plugins/resource/plugin-standard-html.js
index 79317a95..55a0a520 100644
--- a/packages/cli/src/plugins/resource/plugin-standard-html.js
+++ b/packages/cli/src/plugins/resource/plugin-standard-html.js
@@ -92,16 +92,19 @@ const getAppTemplate = (contents, userWorkspace, customImports = []) => {
       const attributes = script.rawAttrs !== '' 
         ? ` ${script.rawAttrs}`
         : '';
+      const source = script.text
+        .replace(/\$/g, '$$$'); // https://github.com/ProjectEvergreen/greenwood/issues/656
+
       if (matchPos > 0) {
         appTemplateContents = sliceTemplate(appTemplateContents, matchPos, matchNeedle, `</script>\n
           <script${attributes}>
-            ${script.text}
+            ${source}
           </script>\n
         `);
       } else {
         appTemplateContents = appTemplateContents.replace('</head>', `
             <script${attributes}>
-              ${script.text}
+              ${source}
             </script>\n
           </head>
         `);
@@ -386,7 +389,7 @@ class StandardHtmlResource extends ResourceInterface {
           contents = contents.replace(/<script defer="" src="(.*es-module-shims.js)"><\/script>/, '');
           contents = contents.replace(/type="module-shim"/g, 'type="module"');
 
-          body = body.replace(/\<head>(.*)<\/head>/s, contents);
+          body = body.replace(/\<head>(.*)<\/head>/s, contents.replace(/\$/g, '$$$')); // https://github.com/ProjectEvergreen/greenwood/issues/656);
         }
     
         resolve(body);


Test patch:
diff --git a/packages/cli/test/cases/build.config.optimization-inline/build.config-optimization-inline.spec.js b/packages/cli/test/cases/build.config.optimization-inline/build.config-optimization-inline.spec.js
index 543f4001..b1193956 100644
--- a/packages/cli/test/cases/build.config.optimization-inline/build.config-optimization-inline.spec.js
+++ b/packages/cli/test/cases/build.config.optimization-inline/build.config-optimization-inline.spec.js
@@ -62,11 +62,11 @@ describe('Build Greenwood With: ', function() {
         expect(linkTags.length).to.be.equal(0);
       });
 
-      describe('<script> tag and files', function() {
-        it('should contain one <script> tags in the <head>', function() {
+      describe('<script> tags and files', function() {
+        it('should contain three <script> tags in the <head>', function() {
           const allScriptTags = dom.window.document.querySelectorAll('head script');
 
-          expect(allScriptTags.length).to.be.equal(1);
+          expect(allScriptTags.length).to.be.equal(3);
         });
 
         it('should contain no <script> tags in the <head> with a src', function() {
@@ -75,13 +75,16 @@ describe('Build Greenwood With: ', function() {
           expect(allSrcScriptTags.length).to.be.equal(0);
         });
       
-        xit('should contain no Javascript files in the output directory', async function() {
+        it('should contain no Javascript files in the output directory', async function() {
           const jsFiles = await glob.promise(`${this.context.publicDir}**/**/*.js`);
           
           expect(jsFiles).to.have.lengthOf(0);
         });
+      });
 
-        it('should contain one <script> tag with the expected JS content inlined of type="module"', function() {
+      // assume the first tag is for the header
+      describe('Header', function() {
+        it('should contain one <script> tag with the expected JS content inlined of type="module" for the header', function() {
           const scriptTag = dom.window.document.querySelectorAll('head script')[0];
           
           expect(scriptTag.type).to.be.equal('module');
@@ -97,8 +100,32 @@ describe('Build Greenwood With: ', function() {
         });
       });
 
+      // assume the second tag is for FooBar
+      // https://github.com/ProjectEvergreen/greenwood/issues/656
+      describe('Foobar', function() {
+        it('should contain one <script> tag with the expected JS content inlined of type="module" for FooBar', function() {
+          const scriptTag = dom.window.document.querySelectorAll('head script')[1];
+
+          expect(scriptTag.type).to.be.equal('module');
+          // eslint-disable-next-line max-len
+          expect(scriptTag.textContent).to.be.contain('class t extends HTMLElement{constructor(){super(),this.list=[]}find(t){this.list.findIndex(e=>new RegExp(`^${t}$`).test(e.route))}}export{t as Foobar};');
+        });
+      });
+
+      // assume the third tag is for Baz
+      // https://github.com/ProjectEvergreen/greenwood/issues/656
+      describe('Baz', function() {
+        it('should contain one <script> tag with the expected JS content for the already inlined of type="module" for Baz', function() {
+          const scriptTag = dom.window.document.querySelectorAll('head script')[2];
+
+          expect(scriptTag.type).to.be.equal('module');
+          // eslint-disable-next-line max-len
+          expect(scriptTag.textContent).to.be.contain('class t extends HTMLElement{constructor(){super(),this.list=[]}find(t){this.list.findIndex(e=>new RegExp(`^${t}$`).test(e.route))}}console.log("1049249078-scratch");export{t as Baz};');
+        });
+      });
+
       describe('<link> tags as <style> tags and file output', function() {
-        xit('should contain no CSS files in the output directory', async function() {
+        it('should contain no CSS files in the output directory', async function() {
           const cssFiles = await glob.promise(`${this.context.publicDir}**/**/*.css`);
           
           expect(cssFiles).to.have.lengthOf(0);
diff --git a/packages/cli/test/cases/build.config.optimization-inline/src/components/foobar.js b/packages/cli/test/cases/build.config.optimization-inline/src/components/foobar.js
new file mode 100644
index 00000000..5e4bb4a9
--- /dev/null
+++ b/packages/cli/test/cases/build.config.optimization-inline/src/components/foobar.js
@@ -0,0 +1,15 @@
+class Foobar extends HTMLElement {
+
+  constructor() {
+    super();
+    this.list = [];
+  }
+
+  find(path) {
+    this.list.findIndex(item => {
+      return new RegExp(`^${path}$`).test(item.route);
+    });
+  }
+}
+
+export { Foobar };
\ No newline at end of file
diff --git a/packages/cli/test/cases/build.config.optimization-inline/src/pages/index.html b/packages/cli/test/cases/build.config.optimization-inline/src/pages/index.html
index 57ef2588..b60546a1 100644
--- a/packages/cli/test/cases/build.config.optimization-inline/src/pages/index.html
+++ b/packages/cli/test/cases/build.config.optimization-inline/src/pages/index.html
@@ -3,6 +3,25 @@
 
   <head>
     <script type="module" src="/components/header.js"></script>
+    <script type="module" src="/components/foobar.js"></script>
+
+    <script type="module">
+      class Baz extends HTMLElement {
+
+        constructor() {
+          super();
+          this.list = [];
+        }
+
+        find(path) {
+          this.list.findIndex(item => {
+            return new RegExp(`^${path}$`).test(item.route);
+          });
+        }
+      }
+
+      export { Baz };
+    </script>
     <link rel="stylesheet" href="/styles/theme.css"></link>  
   </head>