From a445ea9e3fa830aebac2eb7eefe8c23859e0c460 Mon Sep 17 00:00:00 2001 From: cap-Bernardito Date: Thu, 8 Apr 2021 19:15:21 +0300 Subject: [PATCH] fix: memory leak --- README.md | 7 ++-- src/index.js | 4 +++ src/utils.js | 21 +++++++++-- test/__snapshots__/loader.test.js.snap | 29 ++++++++++----- test/fixtures/basic-plugins-2.less | 5 +++ test/fixtures/plugin-2.js | 11 ++++++ test/loader.test.js | 50 ++++++++++++++++++++++++-- 7 files changed, 111 insertions(+), 16 deletions(-) create mode 100644 test/fixtures/basic-plugins-2.less create mode 100644 test/fixtures/plugin-2.js diff --git a/README.md b/README.md index 2474656..fc7f11a 100644 --- a/README.md +++ b/README.md @@ -34,7 +34,8 @@ module.exports = { rules: [ { test: /\.less$/i, - loader: [ // compiles Less to CSS + loader: [ + // compiles Less to CSS "style-loader", "css-loader", "less-loader", @@ -529,13 +530,13 @@ module.exports = { }; ``` -> ℹ️ Access to the [loader context](https://webpack.js.org/api/loaders/#the-loader-context) inside the custom plugin can be done using the `less.webpackLoaderContext` property. +> ℹ️ Access to the [loader context](https://webpack.js.org/api/loaders/#the-loader-context) inside the custom plugin can be done using the `pluginManager.webpackLoaderContext` property. ```js module.exports = { install: function (less, pluginManager, functions) { functions.add("pi", function () { - // Loader context is available in `less.webpackLoaderContext` + // Loader context is available in `pluginManager.webpackLoaderContext` return Math.PI; }); diff --git a/src/index.js b/src/index.js index 69bd347..78b7cde 100644 --- a/src/index.js +++ b/src/index.js @@ -44,6 +44,10 @@ async function lessLoader(source) { return; } + if ("webpackLoaderContext" in less) { + delete less.webpackLoaderContext; + } + const { css, imports } = result; imports.forEach((item) => { diff --git a/src/utils.js b/src/utils.js index 6380eb2..dcd7f40 100644 --- a/src/utils.js +++ b/src/utils.js @@ -1,4 +1,5 @@ import path from "path"; +import util from "util"; import less from "less"; import { klona } from "klona/full"; @@ -181,10 +182,26 @@ function getLessOptions(loaderContext, loaderOptions) { lessOptions.plugins.unshift(createWebpackLessPlugin(loaderContext)); } + const webpackContextDeprecated = util.deprecate( + (context) => context, + "less.webpackLoaderContext is deprecated and will be removed in next major release. Instead use pluginManager.webpackLoaderContext (https://webpack.js.org/loaders/less-loader/#plugins)" + ); + lessOptions.plugins.unshift({ - install(lessProcessor) { + install(lessProcessor, pluginManager) { // eslint-disable-next-line no-param-reassign - lessProcessor.webpackLoaderContext = loaderContext; + pluginManager.webpackLoaderContext = loaderContext; + + // Todo remove in next major release + if (typeof lessProcessor.webpackLoaderContext === "undefined") { + Object.defineProperty(lessProcessor, "webpackLoaderContext", { + configurable: true, + + get() { + return webpackContextDeprecated(loaderContext); + }, + }); + } }, }); diff --git a/test/__snapshots__/loader.test.js.snap b/test/__snapshots__/loader.test.js.snap index 901c3a5..fd31177 100644 --- a/test/__snapshots__/loader.test.js.snap +++ b/test/__snapshots__/loader.test.js.snap @@ -92,26 +92,26 @@ exports[`loader should delegate resolving (LESS) imports with URLs to "less" pac font-family: 'Roboto'; font-style: normal; font-weight: 300; - src: url(https://fonts.gstatic.com/s/roboto/v20/KFOlCnqEu92Fr1MmSU5fBBc9.ttf) format('truetype'); + src: url(https://fonts.gstatic.com/s/roboto/v27/KFOlCnqEu92Fr1MmSU5fBBc9.ttf) format('truetype'); } @font-face { font-family: 'Roboto'; font-style: normal; font-weight: 400; - src: url(https://fonts.gstatic.com/s/roboto/v20/KFOmCnqEu92Fr1Mu4mxP.ttf) format('truetype'); + src: url(https://fonts.gstatic.com/s/roboto/v27/KFOmCnqEu92Fr1Mu4mxP.ttf) format('truetype'); } @font-face { font-family: 'Roboto'; font-style: normal; font-weight: 500; - src: url(https://fonts.gstatic.com/s/roboto/v20/KFOlCnqEu92Fr1MmEU9fBBc9.ttf) format('truetype'); + src: url(https://fonts.gstatic.com/s/roboto/v27/KFOlCnqEu92Fr1MmEU9fBBc9.ttf) format('truetype'); } @font-face { font-family: 'Roboto'; font-style: normal; font-weight: 700; - src: url(https://fonts.gstatic.com/s/roboto/v20/KFOlCnqEu92Fr1MmWUlfBBc9.ttf) format('truetype'); + src: url(https://fonts.gstatic.com/s/roboto/v27/KFOlCnqEu92Fr1MmWUlfBBc9.ttf) format('truetype'); } " `; @@ -157,25 +157,25 @@ exports[`loader should not add to dependencies imports with URLs: css 1`] = ` font-family: 'Roboto'; font-style: normal; font-weight: 300; - src: url(http://fonts.gstatic.com/s/roboto/v20/KFOlCnqEu92Fr1MmSU5fBBc9.ttf) format('truetype'); + src: url(http://fonts.gstatic.com/s/roboto/v27/KFOlCnqEu92Fr1MmSU5fBBc9.ttf) format('truetype'); } @font-face { font-family: 'Roboto'; font-style: normal; font-weight: 300; - src: url(https://fonts.gstatic.com/s/roboto/v20/KFOlCnqEu92Fr1MmSU5fBBc9.ttf) format('truetype'); + src: url(https://fonts.gstatic.com/s/roboto/v27/KFOlCnqEu92Fr1MmSU5fBBc9.ttf) format('truetype'); } @font-face { font-family: 'Roboto'; font-style: normal; font-weight: 400; - src: url(https://fonts.gstatic.com/s/roboto/v20/KFOmCnqEu92Fr1Mu4mxP.ttf) format('truetype'); + src: url(https://fonts.gstatic.com/s/roboto/v27/KFOmCnqEu92Fr1Mu4mxP.ttf) format('truetype'); } @font-face { font-family: 'Roboto'; font-style: normal; font-weight: 500; - src: url(https://fonts.gstatic.com/s/roboto/v20/KFOlCnqEu92Fr1MmEU9fBBc9.ttf) format('truetype'); + src: url(https://fonts.gstatic.com/s/roboto/v27/KFOlCnqEu92Fr1MmEU9fBBc9.ttf) format('truetype'); } " `; @@ -579,6 +579,17 @@ exports[`loader should work lessOptions.relativeUrls is true: errors 1`] = `Arra exports[`loader should work lessOptions.relativeUrls is true: warnings 1`] = `Array []`; +exports[`loader should work loaderContext in less plugins 2: css 1`] = ` +".webpackLoaderContext { + isDefined: true; +} +" +`; + +exports[`loader should work loaderContext in less plugins 2: errors 1`] = `Array []`; + +exports[`loader should work loaderContext in less plugins 2: warnings 1`] = `Array []`; + exports[`loader should work loaderContext in less plugins: css 1`] = ` ".webpackLoaderContext { isDefined: true; @@ -598,7 +609,7 @@ exports[`loader should work third-party plugins as fileLoader: css 1`] = ` font-family: 'Roboto'; font-style: normal; font-weight: 500; - src: url(https://fonts.gstatic.com/s/roboto/v20/KFOlCnqEu92Fr1MmEU9fBBc9.ttf) format('truetype'); + src: url(https://fonts.gstatic.com/s/roboto/v27/KFOlCnqEu92Fr1MmEU9fBBc9.ttf) format('truetype'); } .modules-dir-scope-module { color: hotpink; diff --git a/test/fixtures/basic-plugins-2.less b/test/fixtures/basic-plugins-2.less new file mode 100644 index 0000000..a2cfe01 --- /dev/null +++ b/test/fixtures/basic-plugins-2.less @@ -0,0 +1,5 @@ +@plugin "plugin-2"; + +.webpackLoaderContext { + isDefined: run(); +} diff --git a/test/fixtures/plugin-2.js b/test/fixtures/plugin-2.js new file mode 100644 index 0000000..5ff4714 --- /dev/null +++ b/test/fixtures/plugin-2.js @@ -0,0 +1,11 @@ +registerPlugin({ + install: function(less, pluginManager, functions) { + functions.add('run', function() { + if (typeof pluginManager.webpackLoaderContext !== 'undefined') { + return 'true'; + } + + return 'false'; + }); + } +}) diff --git a/test/loader.test.js b/test/loader.test.js index 2629e97..af0a3a7 100644 --- a/test/loader.test.js +++ b/test/loader.test.js @@ -802,7 +802,7 @@ describe("loader", () => { constructor(less) { super(); - if (typeof less.webpackLoaderContext !== "undefined") { + if (typeof less.webpackLoaderContext.version !== "undefined") { contextInClass = true; } } @@ -817,7 +817,7 @@ describe("loader", () => { const customObjectPlugin = { install(less) { - if (typeof less.webpackLoaderContext !== "undefined") { + if (typeof less.webpackLoaderContext.version !== "undefined") { contextInObject = true; } }, @@ -839,6 +839,52 @@ describe("loader", () => { expect(getErrors(stats)).toMatchSnapshot("errors"); }); + it("should work loaderContext in less plugins 2", async () => { + let contextInClass; + let contextInObject; + + // eslint-disable-next-line global-require + class Plugin extends require("less").FileManager { + constructor(less, pluginManager) { + super(); + + if (typeof pluginManager.webpackLoaderContext !== "undefined") { + contextInClass = true; + } + } + } + + class CustomClassPlugin { + // eslint-disable-next-line class-methods-use-this + install(less, pluginManager) { + pluginManager.addFileManager(new Plugin(less, pluginManager)); + } + } + + const customObjectPlugin = { + install(less, packageManager) { + if (typeof packageManager.webpackLoaderContext !== "undefined") { + contextInObject = true; + } + }, + }; + + const testId = "./basic-plugins-2.less"; + const compiler = getCompiler(testId, { + lessOptions: { + plugins: [new CustomClassPlugin(), customObjectPlugin], + }, + }); + const stats = await compile(compiler); + const codeFromBundle = getCodeFromBundle(stats, compiler); + + expect(contextInClass).toBe(true); + expect(contextInObject).toBe(true); + expect(codeFromBundle.css).toMatchSnapshot("css"); + expect(getWarnings(stats)).toMatchSnapshot("warnings"); + expect(getErrors(stats)).toMatchSnapshot("errors"); + }); + // TODO bug on windows it.skip("should work with circular imports", async () => { const testId = "./circular.less";