feat!: prefer sass-embedded over sass by default (#1211)

Author: G-Rath

README.md

@@ -48,16 +48,12 @@ This allows you to control the versions of all your dependencies, and to choose
 
 > [!NOTE]
 >
-> We highly recommend using [Dart Sass](https://github.com/sass/dart-sass).
+> We highly recommend using [Sass Embedded](https://github.com/sass/embedded-host-node) or [Dart Sass](https://github.com/sass/dart-sass).
 
 > [!WARNING]
 >
 > [Node Sass](https://github.com/sass/node-sass) does not work with [Yarn PnP](https://classic.yarnpkg.com/en/docs/pnp/) and doesn't support [@use rule](https://sass-lang.com/documentation/at-rules/use).
 
-> [!WARNING]
->
-> [Sass Embedded](https://github.com/sass/embedded-host-node) is experimental and in `beta`, therefore some features may not work
-
 Chain the `sass-loader` with the [css-loader](https://github.com/webpack-contrib/css-loader) and the [style-loader](https://github.com/webpack-contrib/style-loader) to immediately apply all styles to the DOM or the [mini-css-extract-plugin](https://github.com/webpack-contrib/mini-css-extract-plugin) to extract it into a separate file.
 
 Then add the loader to your webpack configuration. For example:
@@ -164,8 +160,8 @@ Default: `sass`
 
 The special `implementation` option determines which implementation of Sass to use.
 
-By default, the loader resolve the implementation based on your dependencies.
-Just add the desired implementation to your `package.json` (`sass` or `node-sass` package) and install dependencies.
+By default, the loader resolves the implementation based on your dependencies.
+Just add the desired implementation to your `package.json` (`sass`, `sass-embedded`, or `node-sass` package) and install dependencies.
 
 Example where the `sass-loader` loader uses the `sass` (`dart-sass`) implementation:
 
@@ -193,14 +189,38 @@ Example where the `sass-loader` loader uses the `node-sass` implementation:
 }
 ```
 
-Beware the situation where both `node-sass` and `sass` are installed! By default, the `sass-loader` prefers `sass`.
-In order to avoid this situation you can use the `implementation` option.
+Example where the `sass-loader` loader uses the `sass-embedded` implementation:
 
-The `implementation` options either accepts `sass` (`Dart Sass`) or `node-sass` as a module.
+**package.json**
+
+```json
+{
+  "devDependencies": {
+    "sass-loader": "^7.2.0",
+    "sass": "^1.22.10"
+  },
+  "optionalDependencies": {
+    "sass-embedded": "^1.70.0"
+  }
+}
+```
+
+> [!NOTE]
+>
+> Using `optionalDependencies` means that `sass-loader` can fallback to `sass`
+> when running on an operating system not supported by `sass-embedded`
+
+Be aware of the order that `sass-loader` will resolve the implementation:
+
+1. `sass-embedded`
+2. `sass`
+3. `node-sass`
+
+You can specify a specific implementation by using the `implementation` option, which accepts one of the above values.
 
 #### `object`
 
-For example, to use Dart Sass, you'd pass:
+For example, to always use Dart Sass, you'd pass:
 
 ```js
 module.exports = {
@@ -214,7 +234,7 @@ module.exports = {
           {
             loader: "sass-loader",
             options: {
-              // Prefer `dart-sass`
+              // Prefer `dart-sass`, even if `sass-embedded` is available
               implementation: require("sass"),
             },
           },
@@ -241,7 +261,7 @@ module.exports = {
           {
             loader: "sass-loader",
             options: {
-              // Prefer `dart-sass`
+              // Prefer `dart-sass`, even if `sass-embedded` is available
               implementation: require.resolve("sass"),
             },
           },

src/utils.js

@@ -5,15 +5,15 @@ function getDefaultSassImplementation() {
   let sassImplPkg = "sass";
 
   try {
-    require.resolve("sass");
+    require.resolve("sass-embedded");
+    sassImplPkg = "sass-embedded";
   } catch (ignoreError) {
     try {
-      require.resolve("node-sass");
-      sassImplPkg = "node-sass";
+      require.resolve("sass");
     } catch (_ignoreError) {
       try {
-        require.resolve("sass-embedded");
-        sassImplPkg = "sass-embedded";
+        require.resolve("node-sass");
+        sassImplPkg = "node-sass";
       } catch (__ignoreError) {
         sassImplPkg = "sass";
       }
@@ -676,6 +676,7 @@ function getModernWebpackImporter(loaderContext, implementation, loadPaths) {
       }
 
       try {
+        // eslint-disable-next-line no-shadow
         const contents = await new Promise((resolve, reject) => {
           // Old version of `enhanced-resolve` supports only path as a string
           // TODO simplify in the next major release and pass URL

test/__snapshots__/implementation-option.test.js.snap

@@ -51,7 +51,7 @@ exports[`implementation option not specify: warnings 1`] = `[]`;
 exports[`implementation option should not swallow an error when trying to load a sass implementation: errors 1`] = `
 [
   "ModuleBuildError: Module build failed (from ../src/cjs.js):
-Some error",
+Some error sass-embedded",
 ]
 `;
 
@@ -104,7 +104,7 @@ exports[`implementation option should throw error when unresolved package: warni
 exports[`implementation option should try to load using valid order: errors 1`] = `
 [
   "ModuleBuildError: Module build failed (from ../src/cjs.js):
-Some error sass",
+Some error sass-embedded",
 ]
 `;
 

test/implementation-option.test.js

@@ -182,9 +182,11 @@ describe("implementation option", () => {
     expect(getWarnings(stats)).toMatchSnapshot("warnings");
     expect(getErrors(stats)).toMatchSnapshot("errors");
 
+    expect(sassEmbeddedSpy).toHaveBeenCalledTimes(1);
     expect(nodeSassSpy).toHaveBeenCalledTimes(0);
-    expect(dartSassSpy).toHaveBeenCalledTimes(1);
+    expect(dartSassSpy).toHaveBeenCalledTimes(0);
 
+    sassEmbeddedSpy.mockClear();
     nodeSassSpy.mockClear();
     dartSassSpy.mockClear();
 
@@ -206,9 +208,11 @@ describe("implementation option", () => {
     expect(getWarnings(stats)).toMatchSnapshot("warnings");
     expect(getErrors(stats)).toMatchSnapshot("errors");
 
+    expect(sassEmbeddedSpy).toHaveBeenCalledTimes(1);
     expect(nodeSassSpy).toHaveBeenCalledTimes(0);
-    expect(dartSassSpy).toHaveBeenCalledTimes(1);
+    expect(dartSassSpy).toHaveBeenCalledTimes(0);
 
+    sassEmbeddedSpy.mockClear();
     nodeSassSpy.mockClear();
     dartSassSpy.mockClear();
 
@@ -230,9 +234,11 @@ describe("implementation option", () => {
     expect(getWarnings(stats)).toMatchSnapshot("warnings");
     expect(getErrors(stats)).toMatchSnapshot("errors");
 
+    expect(sassEmbeddedSpyModernAPI).toHaveBeenCalledTimes(1);
     expect(nodeSassSpy).toHaveBeenCalledTimes(0);
-    expect(dartSassSpyModernAPI).toHaveBeenCalledTimes(1);
+    expect(dartSassSpyModernAPI).toHaveBeenCalledTimes(0);
 
+    sassEmbeddedSpyModernAPI.mockClear();
     nodeSassSpy.mockClear();
     dartSassSpyModernAPI.mockClear();
 
@@ -254,9 +260,11 @@ describe("implementation option", () => {
     expect(getWarnings(stats)).toMatchSnapshot("warnings");
     expect(getErrors(stats)).toMatchSnapshot("errors");
 
+    expect(sassEmbeddedCompilerSpies.compileStringSpy).toHaveBeenCalledTimes(1);
+    expect(sassEmbeddedSpyModernAPI).toHaveBeenCalledTimes(0);
     expect(nodeSassSpy).toHaveBeenCalledTimes(0);
     expect(dartSassSpyModernAPI).toHaveBeenCalledTimes(0);
-    expect(dartSassCompilerSpies.compileStringSpy).toHaveBeenCalledTimes(1);
+    expect(dartSassCompilerSpies.compileStringSpy).toHaveBeenCalledTimes(0);
 
     nodeSassSpy.mockClear();
     dartSassSpyModernAPI.mockClear();
@@ -265,7 +273,7 @@ describe("implementation option", () => {
     await close(compiler);
   });
 
-  it.each(["dart-sass", "sass-embedded"])(
+  it.each(["sass-embedded", "dart-sass"])(
     "should support switching the implementation within the same process when using the modern-compiler API",
     async (implementationName) => {
       const testId = getTestId("language", "scss");

test/validate-options.test.js

@@ -9,12 +9,13 @@ describe("validate options", () => {
   const tests = {
     implementation: {
       success: [
+        // eslint-disable-next-line global-require
+        require("sass-embedded"),
         // eslint-disable-next-line global-require
         require("sass"),
         // eslint-disable-next-line global-require
         require("node-sass"),
-        // eslint-disable-next-line global-require
-        require("sass-embedded"),
+        "sass-embedded",
         "sass",
         "node-sass",
       ],