Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: prefer sass-embedded if available #1211

Merged
merged 1 commit into from
Jul 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 33 additions & 13 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:

Expand Down Expand Up @@ -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 = {
Expand All @@ -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"),
},
},
Expand All @@ -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"),
},
},
Expand Down
11 changes: 6 additions & 5 deletions src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@
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");

Check warning on line 12 in src/utils.js

View check run for this annotation

Codecov / codecov/patch

src/utils.js#L12

Added line #L12 was not covered by tests
} catch (_ignoreError) {
try {
require.resolve("sass-embedded");
sassImplPkg = "sass-embedded";
require.resolve("node-sass");
sassImplPkg = "node-sass";

Check warning on line 16 in src/utils.js

View check run for this annotation

Codecov / codecov/patch

src/utils.js#L15-L16

Added lines #L15 - L16 were not covered by tests
} catch (__ignoreError) {
sassImplPkg = "sass";
}
Expand Down Expand Up @@ -676,6 +676,7 @@
}

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
Expand Down
4 changes: 2 additions & 2 deletions test/__snapshots__/implementation-option.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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",
]
`;

Expand Down Expand Up @@ -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",
]
`;

Expand Down
18 changes: 13 additions & 5 deletions test/implementation-option.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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();

Expand All @@ -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();

Expand All @@ -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();
Expand All @@ -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");
Expand Down
5 changes: 3 additions & 2 deletions test/validate-options.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
Expand Down