Skip to content

Commit 8966287

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 9680de8 commit 8966287

3 files changed

Lines changed: 110 additions & 10 deletions

File tree

src/index.js

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,11 @@ async function lessLoader(source) {
3232
return;
3333
}
3434

35-
const lessOptions = getLessOptions(this, options, implementation);
35+
const { lessOptions, pendingDependencyTasks } = getLessOptions(
36+
this,
37+
options,
38+
implementation,
39+
);
3640
const useSourceMap =
3741
typeof options.sourceMap === "boolean" ? options.sourceMap : this.sourceMap;
3842

@@ -93,6 +97,10 @@ async function lessLoader(source) {
9397
this.addDependency(path.normalize(error.filename));
9498
}
9599

100+
// Wait for any pending sync-load dependency tracking so the failed
101+
// build still snapshots the files it touched.
102+
await Promise.all(pendingDependencyTasks);
103+
96104
callback(errorFactory(error));
97105

98106
return;
@@ -104,6 +112,10 @@ async function lessLoader(source) {
104112
delete lessOptions.pluginManager;
105113
}
106114

115+
// Ensure dependencies for any synchronously loaded resources (e.g.
116+
// `data-uri()`, `@plugin`) are tracked before the loader completes.
117+
await Promise.all(pendingDependencyTasks);
118+
107119
const { css, imports } = result;
108120

109121
for (const item of imports) {

src/utils.js

Lines changed: 75 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,14 @@ const MODULE_REQUEST_REGEX = /^[^?]*~/;
2727
*
2828
* @param {LoaderContext} loaderContext
2929
* @param {object} implementation
30+
* @param {Array<Promise<void>>} pendingDependencyTasks
3031
* @returns {LessPlugin}
3132
*/
32-
function createWebpackLessPlugin(loaderContext, implementation) {
33+
function createWebpackLessPlugin(
34+
loaderContext,
35+
implementation,
36+
pendingDependencyTasks,
37+
) {
3338
const lessOptions = loaderContext.getOptions();
3439
const resolve = loaderContext.getResolve({
3540
dependencyType: "less",
@@ -53,13 +58,62 @@ function createWebpackLessPlugin(loaderContext, implementation) {
5358
return true;
5459
}
5560

56-
// Sync resolving is used at least by the `data-uri` function.
57-
// This file manager doesn't know how to do it, so let's delegate it
58-
// to the default file manager of Less.
59-
// We could probably use loaderContext.resolveSync, but it's deprecated,
60-
// see https://webpack.js.org/api/loaders/#this-resolvesync
61+
// Sync loading is used by `data-uri()` and any custom Less function
62+
// (including those installed via `@plugin`). Webpack doesn't expose a
63+
// sync resolver, so we fulfil the sync read by delegating to Less's
64+
// default file manager (which can only handle native filesystem paths)
65+
// and, in parallel, kick off an async webpack resolve so the loaded
66+
// file is tracked as a webpack file dependency. Without this, webpack's
67+
// persistent cache won't invalidate when a sync-loaded file changes.
68+
// See https://github.com/webpack/less-loader/issues/492.
6169
supportsSync() {
62-
return false;
70+
return true;
71+
}
72+
73+
loadFileSync(filename, currentDirectory, options, environment) {
74+
// The default Less `loadFileSync` internally dispatches to
75+
// `this.loadFile` with `options.syncImport = true`. Because we
76+
// override `loadFile` (async), dynamic dispatch would land back in
77+
// our async version and break the sync contract. Invoke the parent
78+
// `loadFile` directly with the sync flag instead.
79+
const result = super.loadFile(
80+
filename,
81+
currentDirectory,
82+
{ ...options, syncImport: true },
83+
environment,
84+
);
85+
86+
if (result && result.filename) {
87+
loaderContext.addDependency(
88+
path.normalize(
89+
path.isAbsolute(result.filename)
90+
? result.filename
91+
: path.resolve(currentDirectory || ".", result.filename),
92+
),
93+
);
94+
}
95+
96+
// Also try to resolve via webpack so aliases / custom resolvers can
97+
// contribute dependencies. The resolved content is discarded - we
98+
// only need the file path to track as a dependency.
99+
pendingDependencyTasks.push(
100+
this.resolveFilename(filename, currentDirectory)
101+
.then((resolved) => {
102+
const absoluteFilename = path.isAbsolute(resolved)
103+
? resolved
104+
: path.resolve(".", resolved);
105+
106+
loaderContext.addDependency(path.normalize(absoluteFilename));
107+
})
108+
.catch(() => {
109+
// Webpack may legitimately fail to resolve paths that Less's
110+
// default sync manager handled (e.g. node-style relative
111+
// lookups). The sync result above is what Less consumes, so
112+
// ignore the async failure.
113+
}),
114+
);
115+
116+
return result;
63117
}
64118

65119
async resolveFilename(filename, currentDirectory) {
@@ -179,6 +233,12 @@ function getLessOptions(loaderContext, loaderOptions, implementation) {
179233
...options,
180234
};
181235

236+
// Collects async dependency-resolution promises kicked off from
237+
// synchronous Less file loads (e.g. `data-uri()`, `@plugin`). The loader
238+
// awaits these before completing so webpack's dependency snapshot is
239+
// accurate.
240+
const pendingDependencyTasks = [];
241+
182242
const plugins = [...lessOptions.plugins];
183243
const shouldUseWebpackImporter =
184244
typeof loaderOptions.webpackImporter === "boolean" ||
@@ -187,7 +247,13 @@ function getLessOptions(loaderContext, loaderOptions, implementation) {
187247
: true;
188248

189249
if (shouldUseWebpackImporter) {
190-
plugins.unshift(createWebpackLessPlugin(loaderContext, implementation));
250+
plugins.unshift(
251+
createWebpackLessPlugin(
252+
loaderContext,
253+
implementation,
254+
pendingDependencyTasks,
255+
),
256+
);
191257
}
192258

193259
plugins.unshift({
@@ -200,7 +266,7 @@ function getLessOptions(loaderContext, loaderOptions, implementation) {
200266

201267
lessOptions.plugins = plugins;
202268

203-
return lessOptions;
269+
return { lessOptions, pendingDependencyTasks };
204270
}
205271

206272
function isUnsupportedUrl(url) {

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)