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: Enable cache with externalDependencies #984

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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 .yarnrc.yml
@@ -1 +1,4 @@
yarnPath: .yarn/releases/yarn-3.2.3.cjs
enableGlobalCache: true
nodeLinker: node-modules
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My prettier-vscode doesn't seem to be working properly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


29 changes: 16 additions & 13 deletions src/cache.js
Expand Up @@ -90,9 +90,16 @@
cacheIdentifier,
cacheDirectory,
cacheCompression,
cachedDepMtimes,
} = params;

const file = path.join(directory, filename(source, cacheIdentifier, options));
const file = path.join(
directory,
filename(source, cacheIdentifier, {
options,
__cachedDepMtimes: cachedDepMtimes,
}),
);

try {
// No errors mean that the file was previously cached
Expand All @@ -119,19 +126,15 @@
// return it to the user asap and write it in cache
const result = await transform(source, options);

// Do not cache if there are external dependencies,
// since they might change and we cannot control it.
if (!result.externalDependencies.length) {
try {
await write(file, cacheCompression, result);
} catch (err) {
if (fallback) {
// Fallback to tmpdir if node_modules folder not writable
return handleCache(os.tmpdir(), params);
}

throw err;
try {
await write(file, cacheCompression, result);
} catch (err) {
if (fallback) {
// Fallback to tmpdir if node_modules folder not writable
return handleCache(os.tmpdir(), params);

Check warning on line 134 in src/cache.js

View check run for this annotation

Codecov / codecov/patch

src/cache.js#L134

Added line #L134 was not covered by tests
}

throw err;

Check warning on line 137 in src/cache.js

View check run for this annotation

Codecov / codecov/patch

src/cache.js#L137

Added line #L137 was not covered by tests
}

return result;
Expand Down
31 changes: 28 additions & 3 deletions src/index.js
Expand Up @@ -26,6 +26,7 @@ const injectCaller = require("./injectCaller");
const schema = require("./schema");

const { isAbsolute } = require("path");
const fs = require("fs");
const validateOptions = require("schema-utils").validate;

function subscribe(subscriber, metadata, context) {
Expand All @@ -39,19 +40,22 @@ module.exports.custom = makeLoader;

function makeLoader(callback) {
const overrides = callback ? callback(babel) : undefined;
const data = {
cachedDepMtimes: { __proto__: null },
};

return function (source, inputSourceMap) {
// Make the loader async
const callback = this.async();

loader.call(this, source, inputSourceMap, overrides).then(
loader.call(this, source, inputSourceMap, overrides, data).then(
args => callback(null, ...args),
err => callback(err),
);
};
}

async function loader(source, inputSourceMap, overrides) {
async function loader(source, inputSourceMap, overrides, data) {
const filename = this.resourcePath;

let loaderOptions = this.getOptions();
Expand Down Expand Up @@ -185,13 +189,24 @@ async function loader(source, inputSourceMap, overrides) {

let result;
if (cacheDirectory) {
const cachedDepMtimes = [];
for (const dep of Object.keys(data.cachedDepMtimes)) {
let mtime = 0;
try {
mtime = fs.statSync(dep).mtimeMs;
} catch (error) {}
cachedDepMtimes.push(dep + mtime);
}
cachedDepMtimes.sort();

result = await cache({
source,
options,
transform,
cacheDirectory,
cacheIdentifier,
cacheCompression,
cachedDepMtimes,
});
} else {
result = await transform(source, options);
Expand All @@ -212,7 +227,17 @@ async function loader(source, inputSourceMap, overrides) {

const { code, map, metadata, externalDependencies } = result;

externalDependencies?.forEach(dep => this.addDependency(dep));
externalDependencies?.forEach(dep => {
if (data.cachedDepMtimes[dep] == null) {
let mtime = 0;
try {
mtime = fs.statSync(dep).mtimeMs;
} catch (error) {}
data.cachedDepMtimes[dep] = mtime;
}
this.addDependency(dep);
});

metadataSubscribers.forEach(subscriber => {
subscribe(subscriber, metadata, this);
});
Expand Down
87 changes: 87 additions & 0 deletions test/cache.test.js
Expand Up @@ -389,3 +389,90 @@ test.cb("should allow to specify the .babelrc file", t => {
});
});
});

test.cb("should cache external dependencies", t => {
const dep = path.join(cacheDir, "externalDependency.txt");

fs.writeFileSync(dep, "123");

let counter = 0;

const config = Object.assign({}, globalConfig, {
entry: path.join(__dirname, "fixtures/constant.js"),
output: {
path: t.context.directory,
},
module: {
rules: [
{
test: /\.js$/,
loader: babelLoader,
options: {
babelrc: false,
configFile: false,
cacheDirectory: t.context.cacheDirectory,
plugins: [
api => {
api.cache.never();
api.addExternalDependency(dep);
return {
visitor: {
BooleanLiteral(path) {
counter++;
path.replaceWith(
api.types.stringLiteral(fs.readFileSync(dep, "utf8")),
);
path.stop();
},
},
};
},
],
},
},
],
},
});

webpack(config, (err, stats) => {
t.deepEqual(stats.compilation.warnings, []);
t.deepEqual(stats.compilation.errors, []);

t.true(stats.compilation.fileDependencies.has(dep));

t.is(counter, 1);

webpack(config, (err, stats) => {
t.deepEqual(stats.compilation.warnings, []);
t.deepEqual(stats.compilation.errors, []);

t.true(stats.compilation.fileDependencies.has(dep));

t.is(counter, 2);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately this still caches twice.


webpack(config, (err, stats) => {
t.deepEqual(stats.compilation.warnings, []);
t.deepEqual(stats.compilation.errors, []);

t.true(stats.compilation.fileDependencies.has(dep));

t.is(counter, 2);

fs.writeFileSync(dep, "456");

setTimeout(() => {
webpack(config, (err, stats) => {
t.deepEqual(stats.compilation.warnings, []);
t.deepEqual(stats.compilation.errors, []);

t.true(stats.compilation.fileDependencies.has(dep));

t.is(counter, 3);

t.end();
});
}, 1000);
});
});
});
});