Fix HTML_JS_REGEX (#986)

* Fix HTML_JS_REGEX

* Add Vue & Svelte tests

snowpack/src/scan-imports.ts

@@ -5,6 +5,7 @@ import * as colors from 'kleur/colors';
 import mime from 'mime-types';
 import path from 'path';
 import stripComments from 'strip-comments';
+import srcFileExtensionMapping from './build/src-file-extension-mapping';
 import {logger} from './logger';
 import {InstallTarget, SnowpackConfig, SnowpackSourceFile} from './types/snowpack';
 import {
@@ -156,10 +157,21 @@ function parseFileForInstallTargets({
   contents,
 }: SnowpackSourceFile): InstallTarget[] {
   try {
-    if (baseExt === '.css' || baseExt === '.scss' || baseExt === '.sass' || baseExt === '.less') {
-      return parseCssForInstallTargets(contents);
-    } else {
-      return parseJsForInstallTargets(contents);
+    switch (baseExt) {
+      case '.css':
+      case '.less':
+      case '.sass':
+      case '.scss': {
+        return parseCssForInstallTargets(contents);
+      }
+      case '.html':
+      case '.svelte':
+      case '.vue': {
+        return parseJsForInstallTargets(extractJSFromHTML({contents, baseExt}));
+      }
+      default: {
+        return parseJsForInstallTargets(contents);
+      }
     }
   } catch (err) {
     // Another error! No hope left, just abort.
@@ -168,6 +180,26 @@ function parseFileForInstallTargets({
   }
 }
 
+/** Extract only JS within <script type="module"> tags (works for .svelte and .vue files, too) */
+function extractJSFromHTML({contents, baseExt}: {contents: string; baseExt: string}): string {
+  // TODO: Replace with matchAll once Node v10 is out of TLS.
+  // const allMatches = [...result.matchAll(new RegExp(HTML_JS_REGEX))];
+  const allMatches: string[][] = [];
+  let match;
+  let regex = new RegExp(HTML_JS_REGEX);
+  if (baseExt === '.svelte' || baseExt === '.vue') {
+    regex = new RegExp(SVELTE_VUE_REGEX); // scan <script> tags, not <script type="module">
+  }
+  while ((match = regex.exec(contents))) {
+    allMatches.push(match);
+  }
+
+  return allMatches
+    .map((match) => match[2]) // match[2] is the code inside the <script></script> element
+    .filter((s) => s.trim())
+    .join('\n');
+}
+
 export function scanDepList(depList: string[], cwd: string): InstallTarget[] {
   return depList
     .map((whitelistItem) => {
@@ -208,55 +240,13 @@ export async function scanImports(cwd: string, config: SnowpackConfig): Promise<
         return null;
       }
 
-      switch (baseExt) {
-        // Probably a license, a README, etc
-        case '': {
-          return null;
-        }
-        // Our import scanner can handle normal JS & even TypeScript without a problem.
-        case '.js':
-        case '.jsx':
-        case '.mjs':
-        case '.ts':
-        case '.css':
-        case '.tsx': {
-          return {
-            baseExt,
-            expandedExt,
-            locOnDisk: filePath,
-            contents: await fs.promises.readFile(filePath, 'utf-8'),
-          };
-        }
-        case '.html':
-        case '.vue':
-        case '.svelte': {
-          const result = await fs.promises.readFile(filePath, 'utf-8');
-          // TODO: Replace with matchAll once Node v10 is out of TLS.
-          // const allMatches = [...result.matchAll(new RegExp(HTML_JS_REGEX))];
-          const allMatches: string[][] = [];
-          let match;
-          let regex = new RegExp(HTML_JS_REGEX);
-          if (baseExt === '.svelte' || baseExt === '.vue') {
-            regex = new RegExp(SVELTE_VUE_REGEX); // scan <script> tags, not <script type="module">
-          }
-          while ((match = regex.exec(result))) {
-            allMatches.push(match);
-          }
-          return {
-            baseExt,
-            expandedExt,
-            locOnDisk: filePath,
-            // match[2] is the code inside the <script></script> element
-            contents: allMatches
-              .map((match) => match[2])
-              .filter((s) => s.trim())
-              .join('\n'),
-          };
-        }
+      // Probably a license, a README, etc
+      if (baseExt === '') {
+        return null;
       }
 
       // If we don't recognize the file type, it could be source. Warn just in case.
-      if (!mime.lookup(baseExt)) {
+      if (!Object.keys(srcFileExtensionMapping).includes(baseExt) && !mime.lookup(baseExt)) {
         logger.warn(
           colors.dim(
             `ignoring unsupported file "${path
@@ -265,9 +255,16 @@ export async function scanImports(cwd: string, config: SnowpackConfig): Promise<
           ),
         );
       }
-      return null;
+
+      return {
+        baseExt,
+        expandedExt,
+        locOnDisk: filePath,
+        contents: await fs.promises.readFile(filePath, 'utf-8'),
+      };
     }),
   );
+
   return scanImportsFromFiles(loadedFiles.filter(isTruthy), config);
 }
 

snowpack/src/util.ts

@@ -30,9 +30,9 @@ const LOCKFILE_HASH_FILE = '.hash';
 
 export const HAS_CDN_HASH_REGEX = /\-[a-zA-Z0-9]{16,}/;
 // NOTE(fks): Must match empty script elements to work properly.
-export const HTML_JS_REGEX = /(<script.*?type="?module"?.*?>)(.*?)<\/script>/gms;
+export const HTML_JS_REGEX = /(<script[^>]*?type="module".*?>)(.*?)<\/script>/gims;
 export const CSS_REGEX = /@import\s*['"](.*)['"];/gs;
-export const SVELTE_VUE_REGEX = /(<script[^>]*>)(.*?)<\/script>/gms;
+export const SVELTE_VUE_REGEX = /(<script[^>]*>)(.*?)<\/script>/gims;
 
 export const URL_HAS_PROTOCOL_REGEX = /^(\w+:)?\/\//;
 

test/build/__snapshots__/build.test.js.snap

@@ -6428,12 +6428,26 @@ exports[`snowpack build resolve-imports: _dist_/index.html 1`] = `
       import components___ from './components/index.js'; // bare import using alias
       import components____ from './components/index.js'; // bare import using alias and index appended
       import components_____ from './components/index.js'; // bare import using alias and index.js appended
-      console.log(components, components_, components__, components___, components____, components_____);
+      console.log(
+        components,
+        components_,
+        components__,
+        components___,
+        components____,
+        components_____,
+      );
       // Importing something that isn't JS
       import styles from './components/style.css.proxy.js'; // relative import
       import styles_ from './components/style.css.proxy.js'; // relative import
       console.log(styles, styles_);
     </script>
+    <!-- exception test 1: comments should be ignored -->
+    <!-- <script type=\\"module\\" src=\\"preact\\"></script> -->
+    <!-- exception test 2: ignore script tags that aren’t type=\\"module\\" -->
+    <script src=\\"svelte\\"></script>
+    <!-- exception test 3: obviously ignore HTML (this tests our RegEx) -->
+    <pre><code>import React from 'react';</code></pre>
+    <script></script>
   </body>
 </html>"
 `;

test/build/config-out/TEST_BUILD_OUT/src/index.js

@@ -1 +1 @@
-console.log("fooey");
+console.log('fooey');

test/build/resolve-imports/src/index.html

@@ -21,18 +21,36 @@
       console.log(sort, sort_, sort__);
 
       // Importing a directory index.js file
-      import components from './components'; // relative import 
+      import components from './components'; // relative import
       import components_ from './components/index'; // relative import with index appended
       import components__ from './components/index.js'; // relative import with index appended
       import components___ from '@app/components'; // bare import using alias
       import components____ from '@app/components/index'; // bare import using alias and index appended
       import components_____ from '@app/components/index.js'; // bare import using alias and index.js appended
-      console.log(components, components_, components__, components___, components____, components_____);
+      console.log(
+        components,
+        components_,
+        components__,
+        components___,
+        components____,
+        components_____,
+      );
 
       // Importing something that isn't JS
-      import styles from './components/style.css'; // relative import 
-      import styles_ from '@app/components/style.css'; // relative import 
+      import styles from './components/style.css'; // relative import
+      import styles_ from '@app/components/style.css'; // relative import
       console.log(styles, styles_);
     </script>
+
+    <!-- exception test 1: comments should be ignored -->
+    <!-- <script type="module" src="preact"></script> -->
+
+    <!-- exception test 2: ignore script tags that aren’t type="module" -->
+    <script src="svelte"></script>
+
+    <!-- exception test 3: obviously ignore HTML (this tests our RegEx) -->
+    <pre><code>import React from 'react';</code></pre>
+
+    <script></script>
   </body>
-</html>
+</html>

test/integration/include/dir/App.svelte

@@ -1,6 +1,9 @@
 <script>
   import { onMount } from "svelte";
-  const message = "Learn Svelte";
+	const message = "Learn Svelte";
+
+	// comments shouldn’t be included
+	// import bootstrap from 'bootstrap';
 
   onMount(() => {
     console.log("MOUNTED");
@@ -58,5 +61,11 @@
 		>
 			{message}
 		</a>
+
+		<!-- test that this comment isn’t imported: -->
+		<!-- import preact from 'preact' -->
+
+		<!-- this also shouldn’t be imported: -->
+		<pre><code>import { derived } from 'svelte/store';</code></pre>
 	</header>
 </div>

test/integration/include/dir/App.vue

@@ -1,3 +1,14 @@
 <script>
-import VueRouter from 'vue-router';
+  import VueRouter from 'vue-router';
+
+  // comments shouldn’t be included
+  // import rxjs from 'rjxs';
 </script>
+
+<div class="App">
+  <!-- test that this comment isn’t imported: -->
+  <!-- import React from 'react' -->
+
+  <!-- this also shouldn’t be imported: -->
+  <pre><code>import xstate from 'xstate';</code></pre>
+</div>