From ad5b19389a1cfc093288ca04a83d2e2cdddd0e29 Mon Sep 17 00:00:00 2001 From: hikiko4ern Date: Thu, 31 Oct 2024 20:59:44 +0700 Subject: [PATCH 1/3] fix(modern-compiler): dispose redundant compilers fixes #1244 --- src/utils.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/utils.js b/src/utils.js index 76d22b36..4ea78ce6 100644 --- a/src/utils.js +++ b/src/utils.js @@ -768,6 +768,8 @@ function getCompileFn(loaderContext, implementation, apiType) { webpackCompiler.hooks.shutdown.tap("sass-loader", () => { compiler.dispose(); }); + } else { + compiler.dispose(); } } From 9db25433e26565dcc407e882d70bb597ba4443eb Mon Sep 17 00:00:00 2001 From: "alexander.akait" Date: Fri, 1 Nov 2024 02:54:05 +0300 Subject: [PATCH 2/3] test: added --- package.json | 2 - ...mentation-option.test.js.no-node-sass.snap | 4 ++ .../implementation-option.test.js.snap | 4 ++ test/implementation-option.test.js | 64 +++++++++++++++++++ 4 files changed, 72 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index fa629c3d..7a9db5f7 100644 --- a/package.json +++ b/package.json @@ -43,7 +43,6 @@ ], "peerDependencies": { "@rspack/core": "0.x || 1.x", - "node-sass": "^4.0.0 || ^5.0.0 || ^6.0.0 || ^7.0.0 || ^8.0.0 || ^9.0.0", "sass": "^1.3.0", "sass-embedded": "*", "webpack": "^5.0.0" @@ -96,7 +95,6 @@ "lint-staged": "^15.2.0", "material-components-web": "^9.0.0", "memfs": "^4.7.7", - "node-sass": "^9.0.0", "node-sass-glob-importer": "^5.3.2", "npm-run-all": "^4.1.5", "prettier": "^3.2.2", diff --git a/test/__snapshots__/implementation-option.test.js.no-node-sass.snap b/test/__snapshots__/implementation-option.test.js.no-node-sass.snap index 68f8787b..9ac59734 100644 --- a/test/__snapshots__/implementation-option.test.js.no-node-sass.snap +++ b/test/__snapshots__/implementation-option.test.js.no-node-sass.snap @@ -48,6 +48,10 @@ exports[`implementation option not specify: errors 1`] = `[]`; exports[`implementation option not specify: warnings 1`] = `[]`; +exports[`implementation option should dispose redundant compilers for \`modern-compiler\`: errors 1`] = `[]`; + +exports[`implementation option should dispose redundant compilers for \`modern-compiler\`: 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): diff --git a/test/__snapshots__/implementation-option.test.js.snap b/test/__snapshots__/implementation-option.test.js.snap index 21d95e88..6f8b6c18 100644 --- a/test/__snapshots__/implementation-option.test.js.snap +++ b/test/__snapshots__/implementation-option.test.js.snap @@ -52,6 +52,10 @@ exports[`implementation option not specify: errors 1`] = `[]`; exports[`implementation option not specify: warnings 1`] = `[]`; +exports[`implementation option should dispose redundant compilers for \`modern-compiler\`: errors 1`] = `[]`; + +exports[`implementation option should dispose redundant compilers for \`modern-compiler\`: 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): diff --git a/test/implementation-option.test.js b/test/implementation-option.test.js index ba11835c..5e699544 100644 --- a/test/implementation-option.test.js +++ b/test/implementation-option.test.js @@ -398,6 +398,70 @@ describe("implementation option", () => { await close(compiler); }); + it("should dispose redundant compilers for `modern-compiler`", async () => { + sassEmbeddedCompilerSpies.mockRestore(); + + let isInRace = false; + + let firstDisposeSpy; + let secondDisposeSpy; + + const actualFn = sassEmbedded.initAsyncCompiler.bind(sassEmbedded); + + const initSpy = jest + .spyOn(sassEmbedded, "initAsyncCompiler") + .mockImplementation(async () => { + const compiler = await actualFn(); + + if (!isInRace) { + firstDisposeSpy = jest.spyOn(compiler, "dispose"); + isInRace = true; + + return new Promise((resolve) => { + const interval = setInterval(() => { + if (!isInRace) { + clearInterval(interval); + resolve(compiler); + } + }); + }); + } + + isInRace = false; + secondDisposeSpy = jest.spyOn(compiler, "dispose"); + + return compiler; + }); + + const testId1 = getTestId("language", "scss"); + const testId2 = getTestId("language", "sass"); + const options = { api: "modern-compiler" }; + + // eslint-disable-next-line no-undefined + const compiler = getCompiler(undefined, { + entry: { + one: `./${testId1}`, + two: `./${testId2}`, + }, + loader: { options }, + }); + const stats = await compile(compiler); + + expect(getWarnings(stats)).toMatchSnapshot("warnings"); + expect(getErrors(stats)).toMatchSnapshot("errors"); + expect(initSpy).toHaveBeenCalledTimes(2); + + await close(compiler); + + initSpy.mockRestore(); + + expect(firstDisposeSpy).toHaveBeenCalledTimes(1); + firstDisposeSpy.mockRestore(); + + expect(secondDisposeSpy).toHaveBeenCalledTimes(1); + secondDisposeSpy.mockRestore(); + }); + it("should try to load using valid order", async () => { jest.doMock("sass", () => { const error = new Error("Some error sass"); From 6ad4ecbb242d512da7d0fe9ac073164dfbec9680 Mon Sep 17 00:00:00 2001 From: "alexander.akait" Date: Fri, 1 Nov 2024 02:58:43 +0300 Subject: [PATCH 3/3] test: fix --- package.json | 2 ++ 1 file changed, 2 insertions(+) diff --git a/package.json b/package.json index 7a9db5f7..fa629c3d 100644 --- a/package.json +++ b/package.json @@ -43,6 +43,7 @@ ], "peerDependencies": { "@rspack/core": "0.x || 1.x", + "node-sass": "^4.0.0 || ^5.0.0 || ^6.0.0 || ^7.0.0 || ^8.0.0 || ^9.0.0", "sass": "^1.3.0", "sass-embedded": "*", "webpack": "^5.0.0" @@ -95,6 +96,7 @@ "lint-staged": "^15.2.0", "material-components-web": "^9.0.0", "memfs": "^4.7.7", + "node-sass": "^9.0.0", "node-sass-glob-importer": "^5.3.2", "npm-run-all": "^4.1.5", "prettier": "^3.2.2",