Skip to content

Commit 2c30390

Browse files
committed
fix: track sync-loaded files as webpack dependencies
Less's `data-uri()` built-in and any custom Less function (including those installed via `@plugin`) load files synchronously. Webpack no longer exposes a sync resolver, so the file manager previously signalled `supportsSync: false` and let Less's default file manager handle the sync read - meaning those files were never added to webpack's file dependency set. With persistent caching enabled, changes to a file used only via `data-uri()` would not invalidate the cached bundle. The file manager now fulfils sync reads itself (delegating the actual read to the parent class's `loadFile` with `syncImport: true`), records the resolved filename as a dependency, and kicks off an async webpack resolve in parallel so aliased / webpack-only resolutions also feed into the dependency set. The loader awaits these tasks before completing. Closes #492
1 parent e112e32 commit 2c30390

3 files changed

Lines changed: 119 additions & 11 deletions

File tree

src/index.js

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,11 @@ async function lessLoader(source) {
4444
return;
4545
}
4646

47-
const lessOptions = getLessOptions(this, options, implementation);
47+
const { lessOptions, pendingDependencyTasks } = getLessOptions(
48+
this,
49+
options,
50+
implementation,
51+
);
4852
const useSourceMap =
4953
typeof options.sourceMap === "boolean" ? options.sourceMap : this.sourceMap;
5054

@@ -111,6 +115,10 @@ async function lessLoader(source) {
111115
this.addDependency(path.normalize(lessError.filename));
112116
}
113117

118+
// Wait for any pending sync-load dependency tracking so the failed
119+
// build still snapshots the files it touched.
120+
await Promise.all(pendingDependencyTasks);
121+
114122
callback(errorFactory(lessError));
115123

116124
return;
@@ -122,6 +130,10 @@ async function lessLoader(source) {
122130
delete lessOptions.pluginManager;
123131
}
124132

133+
// Ensure dependencies for any synchronously loaded resources (e.g.
134+
// `data-uri()`, `@plugin`) are tracked before the loader completes.
135+
await Promise.all(pendingDependencyTasks);
136+
125137
const { css, imports } = result;
126138

127139
for (const item of imports) {

src/utils.js

Lines changed: 84 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,14 @@ const MODULE_REQUEST_REGEX = /^[^?]*~/;
7676
*
7777
* @param {LoaderContext} loaderContext
7878
* @param {Less} implementation
79+
* @param {Array<Promise<void>>} pendingDependencyTasks
7980
* @returns {LessPlugin}
8081
*/
81-
function createWebpackLessPlugin(loaderContext, implementation) {
82+
function createWebpackLessPlugin(
83+
loaderContext,
84+
implementation,
85+
pendingDependencyTasks,
86+
) {
8287
const lessOptions =
8388
/** @type {LessLoaderOptions} */
8489
(loaderContext.getOptions());
@@ -108,16 +113,72 @@ function createWebpackLessPlugin(loaderContext, implementation) {
108113
return true;
109114
}
110115

111-
// Sync resolving is used at least by the `data-uri` function.
112-
// This file manager doesn't know how to do it, so let's delegate it
113-
// to the default file manager of Less.
114-
// We could probably use loaderContext.resolveSync, but it's deprecated,
115-
// see https://webpack.js.org/api/loaders/#this-resolvesync
116+
// Sync loading is used by `data-uri()` and any custom Less function
117+
// (including those installed via `@plugin`). Webpack doesn't expose a
118+
// sync resolver, so we fulfil the sync read by delegating to Less's
119+
// default file manager (which can only handle native filesystem paths)
120+
// and, in parallel, kick off an async webpack resolve so the loaded
121+
// file is tracked as a webpack file dependency. Without this, webpack's
122+
// persistent cache won't invalidate when a sync-loaded file changes.
123+
// See https://github.com/webpack/less-loader/issues/492.
116124
/**
117125
* @returns {boolean}
118126
*/
119127
supportsSync() {
120-
return false;
128+
return true;
129+
}
130+
131+
/**
132+
* @param {string} filename
133+
* @param {string} currentDirectory
134+
* @param {{ [key: string]: unknown }} options
135+
* @param {unknown} environment
136+
* @returns {LoadFileResult}
137+
*/
138+
loadFileSync(filename, currentDirectory, options, environment) {
139+
// The default Less `loadFileSync` internally dispatches to
140+
// `this.loadFile` with `options.syncImport = true`. Because we
141+
// override `loadFile` (async), dynamic dispatch would land back in
142+
// our async version and break the sync contract. Invoke the parent
143+
// `loadFile` directly with the sync flag instead.
144+
const result = super.loadFile(
145+
filename,
146+
currentDirectory,
147+
{ ...options, syncImport: true },
148+
environment,
149+
);
150+
151+
if (result && result.filename) {
152+
loaderContext.addDependency(
153+
path.normalize(
154+
path.isAbsolute(result.filename)
155+
? result.filename
156+
: path.resolve(currentDirectory || ".", result.filename),
157+
),
158+
);
159+
}
160+
161+
// Also try to resolve via webpack so aliases / custom resolvers can
162+
// contribute dependencies. The resolved content is discarded - we
163+
// only need the file path to track as a dependency.
164+
pendingDependencyTasks.push(
165+
this.resolveFilename(filename, currentDirectory)
166+
.then((resolved) => {
167+
const absoluteFilename = path.isAbsolute(resolved)
168+
? resolved
169+
: path.resolve(".", resolved);
170+
171+
loaderContext.addDependency(path.normalize(absoluteFilename));
172+
})
173+
.catch(() => {
174+
// Webpack may legitimately fail to resolve paths that Less's
175+
// default sync manager handled (e.g. node-style relative
176+
// lookups). The sync result above is what Less consumes, so
177+
// ignore the async failure.
178+
}),
179+
);
180+
181+
return result;
121182
}
122183

123184
/**
@@ -238,7 +299,7 @@ function createWebpackLessPlugin(loaderContext, implementation) {
238299
* @param {LoaderContext} loaderContext
239300
* @param {LessLoaderOptions} loaderOptions
240301
* @param {Less} implementation
241-
* @returns {LessOptions}
302+
* @returns {{ lessOptions: LessOptions, pendingDependencyTasks: Array<Promise<void>> }}
242303
*/
243304
function getLessOptions(loaderContext, loaderOptions, implementation) {
244305
const options =
@@ -255,6 +316,13 @@ function getLessOptions(loaderContext, loaderOptions, implementation) {
255316
...options,
256317
};
257318

319+
// Collects async dependency-resolution promises kicked off from
320+
// synchronous Less file loads (e.g. `data-uri()`, `@plugin`). The loader
321+
// awaits these before completing so webpack's dependency snapshot is
322+
// accurate.
323+
/** @type {Array<Promise<void>>} */
324+
const pendingDependencyTasks = [];
325+
258326
const plugins = [...lessOptions.plugins];
259327
const shouldUseWebpackImporter =
260328
typeof loaderOptions.webpackImporter === "boolean" ||
@@ -263,7 +331,13 @@ function getLessOptions(loaderContext, loaderOptions, implementation) {
263331
: true;
264332

265333
if (shouldUseWebpackImporter) {
266-
plugins.unshift(createWebpackLessPlugin(loaderContext, implementation));
334+
plugins.unshift(
335+
createWebpackLessPlugin(
336+
loaderContext,
337+
implementation,
338+
pendingDependencyTasks,
339+
),
340+
);
267341
}
268342

269343
plugins.unshift({
@@ -276,7 +350,7 @@ function getLessOptions(loaderContext, loaderOptions, implementation) {
276350

277351
lessOptions.plugins = plugins;
278352

279-
return lessOptions;
353+
return { lessOptions, pendingDependencyTasks };
280354
}
281355

282356
/**

test/loader.test.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,28 @@ describe("loader", { timeout: 30000 }, () => {
5252
t.assert.snapshot(getErrors(stats));
5353
});
5454

55+
it("should track files loaded synchronously by data-uri as dependencies", async () => {
56+
const testId = "./data-uri.less";
57+
const compiler = getCompiler(testId);
58+
const stats = await compile(compiler);
59+
const { fileDependencies } = stats.compilation;
60+
61+
validateDependencies(fileDependencies);
62+
63+
const fixtures = [
64+
path.resolve(__dirname, "fixtures", "data-uri.less"),
65+
path.resolve(__dirname, "fixtures", "resources", "circle.svg"),
66+
];
67+
68+
for (const fixture of fixtures) {
69+
assert.strictEqual(
70+
fileDependencies.has(fixture),
71+
true,
72+
`Expected ${fixture} to be tracked as a file dependency`,
73+
);
74+
}
75+
});
76+
5577
it("should transform urls", async (t) => {
5678
const testId = "./url-path.less";
5779
const compiler = getCompiler(testId);

0 commit comments

Comments
 (0)