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

fix: disabled fiber on node >= 16 #950

Merged
merged 2 commits into from May 13, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .eslintrc.js
@@ -1,4 +1,7 @@
module.exports = {
root: true,
extends: ["@webpack-contrib/eslint-config-webpack", "prettier"],
parserOptions: {
ecmaVersion: 2020,
},
};
2 changes: 1 addition & 1 deletion .github/workflows/nodejs.yml
Expand Up @@ -55,7 +55,7 @@ jobs:
strategy:
matrix:
os: [ubuntu-latest, windows-latest, macos-latest]
node-version: [10.x, 12.x, 14.x]
node-version: [10.x, 12.x, 14.x, 16.x]
webpack-version: [latest]

runs-on: ${{ matrix.os }}
Expand Down
4 changes: 3 additions & 1 deletion README.md
Expand Up @@ -196,7 +196,9 @@ module.exports = {
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.

We automatically inject the [`fibers`](https://github.com/laverdet/node-fibers) package (setup `sassOptions.fiber`) if is possible (i.e. you need install the [`fibers`](https://github.com/laverdet/node-fibers) package).
We automatically inject the [`fibers`](https://github.com/laverdet/node-fibers) package (setup `sassOptions.fiber`) for `Node.js` less v16.0.0 if is possible (i.e. you need install the [`fibers`](https://github.com/laverdet/node-fibers) package).

> Fibers is not compatible with `Node.js` v16.0.0 or later ([see introduction to readme](https://github.com/laverdet/node-fibers)).

**package.json**

Expand Down
18 changes: 9 additions & 9 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -84,7 +84,7 @@
"lint-staged": "^11.0.0",
"material-components-web": "^8.0.0",
"memfs": "^3.2.2",
"node-sass": "^5.0.0",
"node-sass": "^6.0.0",
"node-sass-glob-importer": "^5.3.2",
"npm-run-all": "^4.1.5",
"prettier": "^2.3.0",
Expand Down
9 changes: 8 additions & 1 deletion src/utils.js
Expand Up @@ -88,6 +88,12 @@ function proxyCustomImporters(importers, loaderContext) {
);
}

function isSupportedFibers() {
const [nodeVersion] = process.versions.node.split(".");

return Number(nodeVersion) < 16;
}

/**
* Derives the sass options from the loader context and normalizes its values with sane defaults.
*
Expand Down Expand Up @@ -115,7 +121,7 @@ async function getSassOptions(

const isDartSass = implementation.info.includes("dart-sass");

if (isDartSass) {
if (isDartSass && isSupportedFibers()) {
const shouldTryToResolveFibers = !options.fiber && options.fiber !== false;

if (shouldTryToResolveFibers) {
Expand Down Expand Up @@ -569,4 +575,5 @@ export {
getWebpackImporter,
getRenderFunctionFromSassImplementation,
normalizeSourceMap,
isSupportedFibers,
};
17 changes: 14 additions & 3 deletions test/additionalData-option.test.js
@@ -1,6 +1,7 @@
import nodeSass from "node-sass";
import dartSass from "sass";
import Fiber from "fibers";

import { isSupportedFibers } from "../src/utils";

import {
compile,
Expand All @@ -13,13 +14,23 @@ import {
getWarnings,
} from "./helpers";

let Fiber;
const implementations = [nodeSass, dartSass];
const syntaxStyles = ["scss", "sass"];

describe("additionalData option", () => {
beforeAll(async () => {
if (isSupportedFibers()) {
const { default: fibers } = await import("fibers");
Fiber = fibers;
}
});

beforeEach(() => {
// The `sass` (`Dart Sass`) package modify the `Function` prototype, but the `jest` lose a prototype
Object.setPrototypeOf(Fiber, Function.prototype);
if (isSupportedFibers()) {
// The `sass` (`Dart Sass`) package modify the `Function` prototype, but the `jest` lose a prototype
Object.setPrototypeOf(Fiber, Function.prototype);
}
});

implementations.forEach((implementation) => {
Expand Down
28 changes: 22 additions & 6 deletions test/implementation-option.test.js
@@ -1,6 +1,7 @@
import nodeSass from "node-sass";
import dartSass from "sass";
import Fiber from "fibers";

import { isSupportedFibers } from "../src/utils";

import {
compile,
Expand All @@ -12,15 +13,24 @@ import {
getWarnings,
} from "./helpers";

const implementations = [nodeSass, dartSass];

jest.setTimeout(30000);

let Fiber;
const implementations = [nodeSass, dartSass];

describe("implementation option", () => {
beforeAll(async () => {
if (isSupportedFibers()) {
const { default: fibers } = await import("fibers");
Fiber = fibers;
}
});

beforeEach(() => {
// The `sass` (`Dart Sass`) package modify the `Function` prototype, but the `jest` lose a prototype
Object.setPrototypeOf(Fiber, Function.prototype);
jest.clearAllMocks();
if (isSupportedFibers()) {
// The `sass` (`Dart Sass`) package modify the `Function` prototype, but the `jest` lose a prototype
Object.setPrototypeOf(Fiber, Function.prototype);
}
});

implementations.forEach((implementation) => {
Expand Down Expand Up @@ -51,6 +61,9 @@ describe("implementation option", () => {
expect(nodeSassSpy).toHaveBeenCalledTimes(0);
expect(dartSassSpy).toHaveBeenCalledTimes(1);
}

nodeSassSpy.mockRestore();
dartSassSpy.mockRestore();
});
});

Expand All @@ -72,6 +85,9 @@ describe("implementation option", () => {

expect(nodeSassSpy).toHaveBeenCalledTimes(0);
expect(dartSassSpy).toHaveBeenCalledTimes(1);

nodeSassSpy.mockRestore();
dartSassSpy.mockRestore();
});

it("should throw an error on an unknown sass implementation", async () => {
Expand Down
26 changes: 20 additions & 6 deletions test/loader.test.js
Expand Up @@ -2,9 +2,10 @@ import path from "path";

import nodeSass from "node-sass";
import dartSass from "sass";
import Fiber from "fibers";
import del from "del";

import { isSupportedFibers } from "../src/utils";

import {
compile,
getCodeFromBundle,
Expand All @@ -16,15 +17,25 @@ import {
getWarnings,
} from "./helpers";

jest.setTimeout(60000);

let Fiber;
const implementations = [nodeSass, dartSass];
const syntaxStyles = ["scss", "sass"];

jest.setTimeout(60000);

describe("loader", () => {
beforeAll(async () => {
if (isSupportedFibers()) {
const { default: fibers } = await import("fibers");
Fiber = fibers;
}
});

beforeEach(() => {
// The `sass` (`Dart Sass`) package modify the `Function` prototype, but the `jest` lose a prototype
Object.setPrototypeOf(Fiber, Function.prototype);
if (isSupportedFibers()) {
// The `sass` (`Dart Sass`) package modify the `Function` prototype, but the `jest` lose a prototype
Object.setPrototypeOf(Fiber, Function.prototype);
}
});

implementations.forEach((implementation) => {
Expand Down Expand Up @@ -420,7 +431,10 @@ describe("loader", () => {
implementation: getImplementationByName(implementationName),
};
const compiler = getCompiler(testId, {
loader: { options, resolve: { mainFields: ["custom-sass", "..."] } },
loader: {
options,
resolve: { mainFields: ["custom-sass", "..."] },
},
});
const stats = await compile(compiler);
const codeFromBundle = getCodeFromBundle(stats, compiler);
Expand Down
27 changes: 20 additions & 7 deletions test/sassOptions-option.test.js
Expand Up @@ -4,7 +4,8 @@ import globImporter from "node-sass-glob-importer";
import semver from "semver";
import nodeSass from "node-sass";
import dartSass from "sass";
import Fiber from "fibers";

import { isSupportedFibers } from "../src/utils";

import {
compile,
Expand All @@ -19,15 +20,25 @@ import {
getCompiler,
} from "./helpers";

jest.setTimeout(30000);

let Fiber;
const implementations = [nodeSass, dartSass];
const syntaxStyles = ["scss", "sass"];

jest.setTimeout(30000);

describe("sassOptions option", () => {
beforeAll(async () => {
if (isSupportedFibers()) {
const { default: fibers } = await import("fibers");
Fiber = fibers;
}
});

beforeEach(() => {
// The `sass` (`Dart Sass`) package modify the `Function` prototype, but the `jest` lose a prototype
Object.setPrototypeOf(Fiber, Function.prototype);
if (isSupportedFibers()) {
// The `sass` (`Dart Sass`) package modify the `Function` prototype, but the `jest` lose a prototype
Object.setPrototypeOf(Fiber, Function.prototype);
}
});

implementations.forEach((implementation) => {
Expand Down Expand Up @@ -371,7 +382,8 @@ describe("sassOptions option", () => {

if (
implementationName === "dart-sass" &&
semver.satisfies(process.version, ">= 10")
semver.satisfies(process.version, ">= 10") &&
isSupportedFibers()
) {
expect(dartSassSpy.mock.calls[0][0]).toHaveProperty("fiber");
}
Expand All @@ -398,7 +410,8 @@ describe("sassOptions option", () => {

if (
implementationName === "dart-sass" &&
semver.satisfies(process.version, ">= 10")
semver.satisfies(process.version, ">= 10") &&
isSupportedFibers()
) {
expect(dartSassSpy.mock.calls[0][0]).toHaveProperty("fiber");
}
Expand Down
17 changes: 14 additions & 3 deletions test/sourceMap-options.test.js
Expand Up @@ -3,7 +3,8 @@ import path from "path";

import nodeSass from "node-sass";
import dartSass from "sass";
import Fiber from "fibers";

import { isSupportedFibers } from "../src/utils";

import {
compile,
Expand All @@ -15,13 +16,23 @@ import {
getWarnings,
} from "./helpers";

let Fiber;
const implementations = [nodeSass, dartSass];
const syntaxStyles = ["scss", "sass"];

describe("sourceMap option", () => {
beforeAll(async () => {
if (isSupportedFibers()) {
const { default: fibers } = await import("fibers");
Fiber = fibers;
}
});

beforeEach(() => {
// The `sass` (`Dart Sass`) package modify the `Function` prototype, but the `jest` lose a prototype
Object.setPrototypeOf(Fiber, Function.prototype);
if (isSupportedFibers()) {
// The `sass` (`Dart Sass`) package modify the `Function` prototype, but the `jest` lose a prototype
Object.setPrototypeOf(Fiber, Function.prototype);
}
});

implementations.forEach((implementation) => {
Expand Down
17 changes: 14 additions & 3 deletions test/validate-options.test.js
@@ -1,4 +1,4 @@
import Fiber from "fibers";
import { isSupportedFibers } from "../src/utils";

import {
getCompiler,
Expand All @@ -7,10 +7,21 @@ import {
getImplementationByName,
} from "./helpers/index";

let Fiber;

describe("validate options", () => {
beforeAll(async () => {
if (isSupportedFibers()) {
const { default: fibers } = await import("fibers");
Fiber = fibers;
}
});

beforeEach(() => {
// The `sass` (`Dart Sass`) package modify the `Function` prototype, but the `jest` lose a prototype
Object.setPrototypeOf(Fiber, Function.prototype);
if (isSupportedFibers()) {
// The `sass` (`Dart Sass`) package modify the `Function` prototype, but the `jest` lose a prototype
Object.setPrototypeOf(Fiber, Function.prototype);
}
});

const tests = {
Expand Down