From 382a3ca7ca8b7041712de30ce5ad8e6532944c1b Mon Sep 17 00:00:00 2001 From: Alexey Lavinsky Date: Thu, 10 Jun 2021 11:29:04 -0700 Subject: [PATCH] feat: allow `String` value for the `implementation` option --- README.md | 33 +++++++++++++++++-- src/options.json | 9 ++++- src/utils.js | 12 +++++++ .../implementation-option.test.js.snap | 13 ++++++++ .../validate-options.test.js.snap | 22 +++++++++---- test/helpers/getImplementationByName.js | 2 ++ test/implementation-option.test.js | 25 ++++++++++++-- test/validate-options.test.js | 4 +-- 8 files changed, 105 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 7e32eb2a..1f1f43ea 100644 --- a/README.md +++ b/README.md @@ -121,7 +121,7 @@ Thankfully there are a two solutions to this problem: | Name | Type | Default | Description | | :---------------------------------------: | :------------------: | :-------------------------------------: | :---------------------------------------------------------------- | -| **[`implementation`](#implementation)** | `{Object}` | `sass` | Setup Sass implementation to use. | +| **[`implementation`](#implementation)** | `{Object\|String}` | `sass` | Setup Sass implementation to use. | | **[`sassOptions`](#sassoptions)** | `{Object\|Function}` | defaults values for Sass implementation | Options for Sass. | | **[`sourceMap`](#sourcemap)** | `{Boolean}` | `compiler.devtool` | Enables/Disables generation of source maps. | | **[`additionalData`](#additionaldata)** | `{String\|Function}` | `undefined` | Prepends/Appends `Sass`/`SCSS` code before the actual entry file. | @@ -129,7 +129,7 @@ Thankfully there are a two solutions to this problem: ### `implementation` -Type: `Object` +Type: `Object | String` Default: `sass` The special `implementation` option determines which implementation of Sass to use. @@ -168,6 +168,8 @@ In order to avoid this situation you can use the `implementation` option. The `implementation` options either accepts `sass` (`Dart Sass`) or `node-sass` as a module. +#### Object + For example, to use Dart Sass, you'd pass: ```js @@ -193,6 +195,33 @@ module.exports = { }; ``` +#### String + +For example, to use Dart Sass, you'd pass: + +```js +module.exports = { + module: { + rules: [ + { + test: /\.s[ac]ss$/i, + use: [ + "style-loader", + "css-loader", + { + loader: "sass-loader", + options: { + // Prefer `dart-sass` + implementation: require.resolve("sass"), + }, + }, + ], + }, + ], + }, +}; +``` + Note that when using `sass` (`Dart Sass`), **synchronous compilation is twice as fast as asynchronous compilation** by default, due to the overhead of asynchronous callbacks. To avoid this overhead, you can use the [fibers](https://www.npmjs.com/package/fibers) package to call asynchronous importers from the synchronous code path. diff --git a/src/options.json b/src/options.json index 6de7a821..fb1fa6b8 100644 --- a/src/options.json +++ b/src/options.json @@ -4,7 +4,14 @@ "properties": { "implementation": { "description": "The implementation of the sass to be used (https://github.com/webpack-contrib/sass-loader#implementation).", - "type": "object" + "anyOf": [ + { + "type": "string" + }, + { + "type": "object" + } + ] }, "sassOptions": { "description": "Options for `node-sass` or `sass` (`Dart Sass`) implementation. (https://github.com/webpack-contrib/sass-loader#implementation).", diff --git a/src/utils.js b/src/utils.js index 627d9b56..55a45b26 100644 --- a/src/utils.js +++ b/src/utils.js @@ -38,6 +38,18 @@ function getSassImplementation(loaderContext, implementation) { } } + if (typeof resolvedImplementation === "string") { + try { + // eslint-disable-next-line import/no-dynamic-require, global-require + resolvedImplementation = require(resolvedImplementation); + } catch (error) { + loaderContext.emitError(error); + + // eslint-disable-next-line consistent-return + return; + } + } + const { info } = resolvedImplementation; if (!info) { diff --git a/test/__snapshots__/implementation-option.test.js.snap b/test/__snapshots__/implementation-option.test.js.snap index 20b62220..92c45c43 100644 --- a/test/__snapshots__/implementation-option.test.js.snap +++ b/test/__snapshots__/implementation-option.test.js.snap @@ -12,6 +12,10 @@ exports[`implementation option not specify: errors 1`] = `Array []`; exports[`implementation option not specify: warnings 1`] = `Array []`; +exports[`implementation option sass_string: errors 1`] = `Array []`; + +exports[`implementation option sass_string: warnings 1`] = `Array []`; + exports[`implementation option should not swallow an error when trying to load a sass implementation: errors 1`] = ` Array [ "ModuleError: Module Error (from ../src/cjs.js): @@ -47,3 +51,12 @@ Unknown Sass implementation.", `; exports[`implementation option should throw error when the "info" does not exist: warnings 1`] = `Array []`; + +exports[`implementation option should throw error when unresolved package: errors 1`] = ` +Array [ + "ModuleError: Module Error (from ../src/cjs.js): +(Emitted value instead of an instance of Error) Error: Cannot find module 'unresolved' from 'src/utils.js'", +] +`; + +exports[`implementation option should throw error when unresolved package: warnings 1`] = `Array []`; diff --git a/test/__snapshots__/validate-options.test.js.snap b/test/__snapshots__/validate-options.test.js.snap index 4558faba..91e564ef 100644 --- a/test/__snapshots__/validate-options.test.js.snap +++ b/test/__snapshots__/validate-options.test.js.snap @@ -10,18 +10,26 @@ exports[`validate options should throw an error on the "additionalData" option w * options.additionalData should be an instance of function." `; -exports[`validate options should throw an error on the "implementation" option with "string" value 1`] = ` +exports[`validate options should throw an error on the "implementation" option with "() => {}" value 1`] = ` "Invalid options object. Sass Loader has been initialized using an options object that does not match the API schema. - - options.implementation should be an object: - object { … } - -> The implementation of the sass to be used (https://github.com/webpack-contrib/sass-loader#implementation)." + - options.implementation should be one of these: + string | object { … } + -> The implementation of the sass to be used (https://github.com/webpack-contrib/sass-loader#implementation). + Details: + * options.implementation should be a string. + * options.implementation should be an object: + object { … }" `; exports[`validate options should throw an error on the "implementation" option with "true" value 1`] = ` "Invalid options object. Sass Loader has been initialized using an options object that does not match the API schema. - - options.implementation should be an object: - object { … } - -> The implementation of the sass to be used (https://github.com/webpack-contrib/sass-loader#implementation)." + - options.implementation should be one of these: + string | object { … } + -> The implementation of the sass to be used (https://github.com/webpack-contrib/sass-loader#implementation). + Details: + * options.implementation should be a string. + * options.implementation should be an object: + object { … }" `; exports[`validate options should throw an error on the "sassOptions" option with "string" value 1`] = ` diff --git a/test/helpers/getImplementationByName.js b/test/helpers/getImplementationByName.js index fe6098f9..50a60583 100644 --- a/test/helpers/getImplementationByName.js +++ b/test/helpers/getImplementationByName.js @@ -5,6 +5,8 @@ function getImplementationByName(implementationName) { } else if (implementationName === "dart-sass") { // eslint-disable-next-line global-require return require("sass"); + } else if (implementationName === "sass_string") { + return require.resolve("sass"); } throw new Error(`Can't find ${implementationName}`); diff --git a/test/implementation-option.test.js b/test/implementation-option.test.js index 4d445616..6e970bb2 100644 --- a/test/implementation-option.test.js +++ b/test/implementation-option.test.js @@ -16,7 +16,7 @@ import { jest.setTimeout(30000); let Fiber; -const implementations = [nodeSass, dartSass]; +const implementations = [nodeSass, dartSass, "sass_string"]; describe("implementation option", () => { beforeAll(async () => { @@ -34,7 +34,11 @@ describe("implementation option", () => { }); implementations.forEach((implementation) => { - const [implementationName] = implementation.info.split("\t"); + let implementationName = implementation; + + if (implementation.info) { + [implementationName] = implementation.info.split("\t"); + } it(`${implementationName}`, async () => { const nodeSassSpy = jest.spyOn(nodeSass, "render"); @@ -57,7 +61,10 @@ describe("implementation option", () => { if (implementationName === "node-sass") { expect(nodeSassSpy).toHaveBeenCalledTimes(1); expect(dartSassSpy).toHaveBeenCalledTimes(0); - } else if (implementationName === "dart-sass") { + } else if ( + implementationName === "dart-sass" || + implementationName === "dart-sass_string" + ) { expect(nodeSassSpy).toHaveBeenCalledTimes(0); expect(dartSassSpy).toHaveBeenCalledTimes(1); } @@ -67,6 +74,18 @@ describe("implementation option", () => { }); }); + it("should throw error when unresolved package", async () => { + const testId = getTestId("language", "scss"); + const options = { + implementation: "unresolved", + }; + const compiler = getCompiler(testId, { loader: { options } }); + const stats = await compile(compiler); + + expect(getWarnings(stats)).toMatchSnapshot("warnings"); + expect(getErrors(stats)).toMatchSnapshot("errors"); + }); + it("not specify", async () => { const nodeSassSpy = jest.spyOn(nodeSass, "render"); const dartSassSpy = jest.spyOn(dartSass, "render"); diff --git a/test/validate-options.test.js b/test/validate-options.test.js index 5e542d0b..7f43e0eb 100644 --- a/test/validate-options.test.js +++ b/test/validate-options.test.js @@ -27,8 +27,8 @@ describe("validate options", () => { const tests = { implementation: { // eslint-disable-next-line global-require - success: [require("sass"), require("node-sass")], - failure: [true, "string"], + success: [require("sass"), require("node-sass"), "sass", "node-sass"], + failure: [true, () => {}], }, sassOptions: { success: [