feature #517 Allow to use Dart Sass instead of Node Sass (Lyrkan, weaverryan)

weaverryan · weaverryan · commit 757116494ede · 2019-03-01T09:44:47.000-05:00
This PR was merged into the master branch. Discussion ---------- Allow to use Dart Sass instead of Node Sass This PR closes #422 by allowing `sass` (Dart Sass) as an alternative choice to `node-sass`. --- The `sass-loader` allows to set the implementation that should be used to process Sass files. It currently accepts either `sass` or `node-sass` (default): In theory this can already be used by setting the related option when calling `Encore.enableSassLoader()`, but Encore then throws an error asking you to install `node-sass`, even if you already have `sass` installed. One way to prevent that could be to detect when `options.implementation` is set but in its next version the `sass-loader` will [automatically detect](webpack-contrib/sass-loader@ff90dd6) which implementation is installed and use it accordingly. So, instead of doing that, this PR allows to define alternative packages in our `feature.js` file, for instance: ```js const features = { sass: { method: 'enableSassLoader()', packages: [ { name: 'sass-loader', enforce_version: true }, [{ name: 'node-sass' }, { name: 'sass' }] // Will allow both `node-sass` and `sass` ], description: 'load Sass files' }, // ... } ``` If neither `node-sass` or `sass` is available, the error message will display both choices, and recommend the first one in the `yarn add` advice: ![image](https://user-images.githubusercontent.com/850046/52538612-0338fa00-2d75-11e9-8d89-146fe7ee27eb.png) Note that for now using Dart Sass still requires the `implementation` option to be set: ```js Encore.enableSassLoader(options => { options.implementation = require('sass'); }); ``` This shouldn't be a big issue since the default recommendation is still to use `node-sass`. Commits ------- 4e90bf8 clarifying comment 1346ee5 Allow to use Dart Sass instead of Node Sass
diff --git a/lib/features.js b/lib/features.js
@@ -20,7 +20,8 @@ const features = {
         method: 'enableSassLoader()',
         packages: [
             { name: 'sass-loader', enforce_version: true },
-            { name: 'node-sass' }
+            // allow node-sass or sass to be installed
+            [{ name: 'node-sass' }, { name: 'sass' }]
         ],
         description: 'load Sass files'
     },
diff --git a/lib/package-helper.js b/lib/package-helper.js
@@ -36,18 +36,20 @@ function getInstallCommand(packageConfigs) {
     const hasYarnLockfile = fs.existsSync('yarn.lock');
     const hasNpmLockfile = fs.existsSync('package-lock.json');
     const packageInstallStrings = packageConfigs.map((packageConfig) => {
-        if (typeof packageConfig.version === 'undefined') {
-            return packageConfig.name;
+        const firstPackage = packageConfig[0];
+
+        if (typeof firstPackage.version === 'undefined') {
+            return firstPackage.name;
         }
 
         // e.g. ^4.0||^5.0: use the latest version
-        let recommendedVersion = packageConfig.version;
+        let recommendedVersion = firstPackage.version;
         if (recommendedVersion.indexOf('||') !== -1) {
             recommendedVersion = recommendedVersion.split('|').pop().trim();
         }
 
         // recommend the version included in our package.json file
-        return `${packageConfig.name}@${recommendedVersion}`;
+        return `${firstPackage.name}@${recommendedVersion}`;
     });
 
     if (hasNpmLockfile && !hasYarnLockfile) {
@@ -57,13 +59,32 @@ function getInstallCommand(packageConfigs) {
     return chalk.yellow(`yarn add ${packageInstallStrings.join(' ')} --dev`);
 }
 
+function isPackageInstalled(packageConfig) {
+    try {
+        require.resolve(packageConfig.name);
+        return true;
+    } catch (e) {
+        return false;
+    }
+}
+
+function getPackageVersion(packageConfig) {
+    try {
+        return require(`${packageConfig.name}/package.json`).version;
+    } catch (e) {
+        return null;
+    }
+}
+
 function getMissingPackageRecommendations(packagesConfig, requestedFeature = null) {
     let missingPackageConfigs = [];
 
     for (let packageConfig of packagesConfig) {
-        try {
-            require.resolve(packageConfig.name);
-        } catch (e) {
+        if (!Array.isArray(packageConfig)) {
+            packageConfig = [packageConfig];
+        }
+
+        if (!packageConfig.some(isPackageInstalled)) {
             missingPackageConfigs.push(packageConfig);
         }
     }
@@ -72,8 +93,18 @@ function getMissingPackageRecommendations(packagesConfig, requestedFeature = nul
         return;
     }
 
-    const missingPackageNamesChalked = missingPackageConfigs.map(function(packageConfig) {
-        return chalk.green(packageConfig.name);
+    const missingPackageNamesChalked = missingPackageConfigs.map(function(packageConfigs) {
+        const packageNames = packageConfigs.map(packageConfig => {
+            return chalk.green(packageConfig.name);
+        });
+
+        let missingPackages = packageNames[0];
+        if (packageNames.length > 1) {
+            const alternativePackages = packageNames.slice(1);
+            missingPackages = `${missingPackages} (or ${alternativePackages.join(' or ')})`;
+        }
+
+        return missingPackages;
     });
 
     let message = `Install ${missingPackageNamesChalked.join(' & ')}`;
@@ -89,45 +120,56 @@ function getMissingPackageRecommendations(packagesConfig, requestedFeature = nul
     };
 }
 
-function getInvalidPackageVersionRecommendations(packagesConfig, requestedFeature) {
-    let badVersionMessages = [];
+function getInvalidPackageVersionRecommendations(packagesConfig) {
+    const processPackagesConfig = (packageConfig) => {
+        if (Array.isArray(packageConfig)) {
+            let messages = [];
+
+            for (const config of packageConfig) {
+                messages = messages.concat(processPackagesConfig(config));
+            }
+
+            return messages;
+        }
 
-    for (let packageConfig of packagesConfig) {
         if (typeof packageConfig.version === 'undefined') {
-            continue;
+            return [];
         }
 
-        let version;
-        try {
-            version = require(`${packageConfig.name}/package.json`).version;
-        } catch (e) {
-            // should not happen because this functions is meant to be
-            // called only after verifying a package is actually installed
-            throw new Error(`Could not find package.json file for ${packageConfig.name}`);
+        const version = getPackageVersion(packageConfig);
+
+        // If version is null at this point it should be because
+        // of an optional dependency whose presence has already
+        // been checked before.
+        if (version === null) {
+            return [];
         }
 
         if (semver.satisfies(version, packageConfig.version)) {
-            continue;
+            return [];
         }
 
         if (semver.gtr(version, packageConfig.version)) {
-            badVersionMessages.push(
+            return [
                 `Webpack Encore requires version ${chalk.green(packageConfig.version)} of ${chalk.green(packageConfig.name)}. Your version ${chalk.green(version)} is too new. The related feature *may* still work properly. If you have issues, try downgrading the library, or upgrading Encore.`
-            );
+            ];
         } else {
-            badVersionMessages.push(
+            return [
                 `Webpack Encore requires version ${chalk.green(packageConfig.version)} of ${chalk.green(packageConfig.name)}, but your version (${chalk.green(version)}) is too old. The related feature will probably *not* work correctly.`
-            );
+            ];
         }
-    }
+    };
 
-    return badVersionMessages;
+    return processPackagesConfig(packagesConfig);
 }
 
 function addPackagesVersionConstraint(packages) {
     const packageJsonData = require('../package.json');
+    const addConstraint = (packageData) => {
+        if (Array.isArray(packageData)) {
+            return packageData.map(addConstraint);
+        }
 
-    return packages.map(packageData => {
         const newData = Object.assign({}, packageData);
 
         if (packageData.enforce_version) {
@@ -143,7 +185,10 @@ function addPackagesVersionConstraint(packages) {
         }
 
         return newData;
-    });
+    };
+
+
+    return packages.map(addConstraint);
 }
 
 module.exports = {
diff --git a/package.json b/package.json
@@ -80,8 +80,10 @@
     "postcss-loader": "^3.0.0",
     "preact": "^8.2.1",
     "preact-compat": "^3.17.0",
+    "sass": "^1.17.0",
     "sass-loader": "^7.0.1",
     "sinon": "^2.3.4",
+    "strip-ansi": "^5.0.0",
     "stylus": "^0.54.5",
     "stylus-loader": "^3.0.2",
     "ts-loader": "^5.3.0",
diff --git a/test/functional.js b/test/functional.js
@@ -524,7 +524,10 @@ describe('Functional tests using webpack', function() {
             config.setPublicPath('/build');
             config.addStyleEntry('bg', './css/background_image.scss');
             config.addStyleEntry('font', './css/roboto_font.css');
-            config.enableSassLoader();
+            config.enableSassLoader(options => {
+                // Use sass-loader instead of node-sass
+                options.implementation = require('sass');
+            });
 
             testSetup.runWebpack(config, (webpackAssert) => {
                 expect(config.outputPath).to.be.a.directory()
diff --git a/test/package-helper.js b/test/package-helper.js
@@ -14,6 +14,7 @@ const packageHelper = require('../lib/package-helper');
 const path = require('path');
 const process = require('process');
 const fs = require('fs');
+const stripAnsi = require('strip-ansi');
 
 describe('package-helper', () => {
     const baseCwd = process.cwd();
@@ -29,6 +30,7 @@ describe('package-helper', () => {
                 { name: 'foo' }, { name: 'webpack' }, { name: 'bar' }
             ]);
             expect(packageRecommendations.installCommand).to.contain('yarn add foo bar');
+            expect(stripAnsi(packageRecommendations.message)).to.contain('foo & bar');
         });
 
         it('missing packages with package-lock.json only', () => {
@@ -37,6 +39,7 @@ describe('package-helper', () => {
                 { name: 'foo' }, { name: 'webpack' }, { name: 'bar' }
             ]);
             expect(packageRecommendations.installCommand).to.contain('npm install foo bar');
+            expect(stripAnsi(packageRecommendations.message)).to.contain('foo & bar');
         });
 
         it('missing packages with yarn.lock only', () => {
@@ -45,6 +48,7 @@ describe('package-helper', () => {
                 { name: 'foo' }, { name: 'webpack' }, { name: 'bar' }
             ]);
             expect(packageRecommendations.installCommand).to.contain('yarn add foo bar');
+            expect(stripAnsi(packageRecommendations.message)).to.contain('foo & bar');
         });
 
         it('missing packages with both package-lock.json and yarn.lock', () => {
@@ -53,6 +57,19 @@ describe('package-helper', () => {
                 { name: 'foo' }, { name: 'webpack' }, { name: 'bar' }
             ]);
             expect(packageRecommendations.installCommand).to.contain('yarn add foo bar');
+            expect(stripAnsi(packageRecommendations.message)).to.contain('foo & bar');
+        });
+
+        it('missing packages with alternative packages', () => {
+            process.chdir(path.join(__dirname, '../fixtures/package-helper/yarn'));
+            const packageRecommendations = packageHelper.getMissingPackageRecommendations([
+                { name: 'foo' },
+                [{ name: 'bar' }, { name: 'baz' }],
+                [{ name: 'qux' }, { name: 'corge' }, { name: 'grault' }],
+                [{ name: 'quux' }, { name: 'webpack' }],
+            ]);
+            expect(packageRecommendations.installCommand).to.contain('yarn add foo bar qux');
+            expect(stripAnsi(packageRecommendations.message)).to.contain('foo & bar (or baz) & qux (or corge or grault)');
         });
     });
 
@@ -91,6 +108,16 @@ describe('package-helper', () => {
 
             expect(packageRecommendations.installCommand).to.contain('yarn add foo@^8.0 bar');
         });
+
+        it('Recommends correct install with alternative packages', () => {
+            const packageRecommendations = packageHelper.getMissingPackageRecommendations([
+                { name: 'foo', version: '^7.0 || ^8.0' },
+                [{ name: 'bar' }, { name: 'baz' }],
+                [{ name: 'qux', version: '^1.0' }, { name: 'quux', version: '^2.0' }]
+            ]);
+
+            expect(packageRecommendations.installCommand).to.contain('yarn add foo@^8.0 bar qux@^1.0');
+        });
     });
 
     describe('The getInvalidPackageVersionRecommendations correctly checks installed versions', () => {
diff --git a/yarn.lock b/yarn.lock
@@ -1082,6 +1082,11 @@ ansi-regex@^3.0.0:
   resolved "https://registry.yarnpkg.com/ansi-regex/-/ansi-regex-3.0.0.tgz#ed0317c322064f79466c02966bddb605ab37d998"
   integrity sha1-7QMXwyIGT3lGbAKWa922Bas32Zg=
 
+ansi-regex@^4.0.0:
+  version "4.0.0"
+  resolved "https://registry.yarnpkg.com/ansi-regex/-/ansi-regex-4.0.0.tgz#70de791edf021404c3fd615aa89118ae0432e5a9"
+  integrity sha512-iB5Dda8t/UqpPI/IjsejXu5jOGDrzn41wJyljwPH65VCIbk6+1BzFIMJGFwTNrYXT1CrD+B4l19U7awiQ8rk7w==
+
 ansi-styles@^2.2.1:
   version "2.2.1"
   resolved "https://registry.yarnpkg.com/ansi-styles/-/ansi-styles-2.2.1.tgz#b432dd3358b634cf75e1e4664368240533c1ddbe"
@@ -7548,6 +7553,13 @@ sass-loader@^7.0.1:
     neo-async "^2.5.0"
     pify "^3.0.0"
 
+sass@^1.17.0:
+  version "1.17.0"
+  resolved "https://registry.yarnpkg.com/sass/-/sass-1.17.0.tgz#e370b9302af121c9eadad5639619127772094ae6"
+  integrity sha512-aFi9RQqrCYkHB2DaLKBBbdUhos1N5o3l1ke9N5JqWzgSPmYwZsdmA+ViPVatUy/RPA21uejgYVUXM7GCh8lcdw==
+  dependencies:
+    chokidar "^2.0.0"
+
 sax@0.5.x:
   version "0.5.8"
   resolved "https://registry.yarnpkg.com/sax/-/sax-0.5.8.tgz#d472db228eb331c2506b0e8c15524adb939d12c1"
@@ -8097,6 +8109,13 @@ strip-ansi@^4.0.0:
   dependencies:
     ansi-regex "^3.0.0"
 
+strip-ansi@^5.0.0:
+  version "5.0.0"
+  resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-5.0.0.tgz#f78f68b5d0866c20b2c9b8c61b5298508dc8756f"
+  integrity sha512-Uu7gQyZI7J7gn5qLn1Np3G9vcYGTVqB+lFTytnDJv83dd8T22aGH451P3jueT2/QemInJDfxHB5Tde5OzgG1Ow==
+  dependencies:
+    ansi-regex "^4.0.0"
+
 strip-bom@^2.0.0:
   version "2.0.0"
   resolved "https://registry.yarnpkg.com/strip-bom/-/strip-bom-2.0.0.tgz#6219a85616520491f35788bdbf1447a99c7e6b0e"
