Fix Sass partial changes not triggering recompiles in Dev (#2792)

* recursive sass import scanning

* sass directory index

* change variable type

* refactor away from unsupported flatMap function

* fix circular sets

* move comment to correct location

* comment change

plugins/plugin-sass/plugin.js

@@ -3,14 +3,37 @@ const path = require('path');
 const execa = require('execa');
 const npmRunPath = require('npm-run-path');
 
-const IMPORT_REGEX = /\@(use|import)\s*['"](.*?)['"]/g;
+const IMPORT_REGEX = /\@(use|import|forward)\s*['"](.*?)['"]/g;
 const PARTIAL_REGEX = /([\/\\])_(.+)(?![\/\\])/;
 
 function stripFileExtension(filename) {
   return filename.split('.').slice(0, -1).join('.');
 }
 
-function scanSassImports(fileContents, filePath, fileExt) {
+function findChildPartials(pathName, fileName, fileExt) {
+  const dirPath = path.parse(pathName).dir;
+
+  // Prepend a "_" to signify a partial.
+  if (!fileName.startsWith('_')) {
+    fileName = '_' + fileName;
+  }
+
+  // Add on the file extension if it is not already used.
+  if (!fileName.endsWith('.scss') || !fileName.endsWith('.sass')) {
+    fileName += fileExt;
+  }
+
+  const filePath = path.resolve(dirPath, fileName);
+
+  let contents = '';
+  try {
+    contents = fs.readFileSync(filePath, 'utf8');
+  } catch (err) {}
+
+  return contents;
+}
+
+function scanSassImports(fileContents, filePath, fileExt, partials = new Set()) {
   // TODO: Replace with matchAll once Node v10 is out of TLS.
   // const allMatches = [...result.matchAll(new RegExp(HTML_JS_REGEX))];
   const allMatches = [];
@@ -20,12 +43,38 @@ function scanSassImports(fileContents, filePath, fileExt) {
     allMatches.push(match);
   }
   // return all imports, resolved to true files on disk.
-  return allMatches
+  allMatches
     .map((match) => match[2])
     .filter((s) => s.trim())
-    .map((s) => {
-      return path.resolve(path.dirname(filePath), s);
+    // Avoid node packages and core sass libraries.
+    .filter((s) => !s.includes('node_modules') && !s.includes('sass:'))
+    .forEach((fileName) => {
+      let pathName = path.resolve(path.dirname(filePath), fileName);
+
+      if (partials.has(pathName)) {
+        return;
+      }
+
+      // Add this partial to the main list being passed to avoid duplicates.
+      partials.add(pathName);
+
+      // If it is a directory then look for an _index file.
+      try {
+        if (fs.lstatSync(pathName).isDirectory()) {
+          fileName = 'index';
+          pathName += '/' + fileName;
+        }
+      } catch (err) {}
+
+      // Recursively find any child partials that have not already been added.
+      const partialsContent = findChildPartials(pathName, fileName, fileExt);
+      if (partialsContent) {
+        const childPartials = scanSassImports(partialsContent, pathName, fileExt, partials);
+        partials.add(...childPartials);
+      }
     });
+
+  return partials;
 }
 
 module.exports = function sassPlugin(snowpackConfig, {native, compilerOptions = {}} = {}) {
@@ -96,7 +145,7 @@ module.exports = function sassPlugin(snowpackConfig, {native, compilerOptions =
       // During development, we need to track changes to Sass dependencies.
       if (isDev) {
         const sassImports = scanSassImports(contents, filePath, fileExt);
-        sassImports.forEach((imp) => addImportsToMap(filePath, imp));
+        [...sassImports].forEach((imp) => addImportsToMap(filePath, imp));
       }
 
       // If file is `.sass`, use YAML-style. Otherwise, use default.

plugins/plugin-sass/test/__snapshots__/plugin.test.js.snap

@@ -1,7 +1,11 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
 exports[`plugin-sass returns the compiled Sass result: App.sass 1`] = `
-"body {
+".child {
+  text-align: left;
+}
+
+body {
   font-family: Helvetica, sans-serif;
 }
 

plugins/plugin-sass/test/fixtures/sass/App.sass

@@ -4,6 +4,6 @@
 body
     font-family: folder.$font-stack
 
-.App 
+.App
   text-align: center
-  background: base.$primary-color
+  background: base.$primary-color

plugins/plugin-sass/test/fixtures/sass/folder/_child-partial.sass

@@ -0,0 +1,2 @@
+.child
+  text-align: left

plugins/plugin-sass/test/fixtures/sass/folder/_index.sass

@@ -1,2 +1,4 @@
 // folder/_index.sass
+@use 'child-partial'
+
 $font-stack: Helvetica, sans-serif

plugins/plugin-sass/test/plugin.test.js

@@ -3,6 +3,8 @@ const path = require('path');
 
 const pathToSassApp = path.join(__dirname, 'fixtures/sass/App.sass');
 const pathToSassBase = path.join(__dirname, 'fixtures/sass/_base.sass');
+const pathToSassIndex = path.join(__dirname, 'fixtures/sass/folder/_index.sass');
+const pathToSassChild = path.join(__dirname, 'fixtures/sass/folder/_child-partial.sass');
 const pathToScssApp = path.join(__dirname, 'fixtures/scss/App.scss');
 const pathToBadCode = path.join(__dirname, 'fixtures/bad/bad.scss');
 
@@ -39,6 +41,12 @@ describe('plugin-sass', () => {
     expect(p.markChanged.mock.calls).toEqual([]);
     p.onChange({filePath: pathToSassBase});
     expect(p.markChanged.mock.calls).toEqual([[pathToSassApp]]);
+    p.markChanged.mockClear();
+    p.onChange({filePath: pathToSassIndex});
+    expect(p.markChanged.mock.calls).toEqual([[pathToSassApp]]);
+    p.markChanged.mockClear();
+    p.onChange({filePath: pathToSassChild});
+    expect(p.markChanged.mock.calls).toEqual([[pathToSassApp]]);
   });
 
   test('does not track dependant changes when isDev=false', async () => {