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 1 commit
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
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 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 @@

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 @@

let result;
if (cacheDirectory) {
const cachedDepMtimes = [];
for (const dep of Object.keys(data.cachedDepMtimes)) {
let mtime = 0;
try {
mtime = fs.statSync(dep).mtime;

Check warning on line 196 in src/index.js

View check run for this annotation

Codecov / codecov/patch

src/index.js#L195-L196

Added lines #L195 - L196 were not covered by tests
} catch (error) {}
cachedDepMtimes.push(dep + mtime);

Check warning on line 198 in src/index.js

View check run for this annotation

Codecov / codecov/patch

src/index.js#L198

Added line #L198 was not covered by tests
}
cachedDepMtimes.sort();

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

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).mtime;
} catch (error) {}
data.cachedDepMtimes[dep] = mtime;
}
this.addDependency(dep);
});

metadataSubscribers.forEach(subscriber => {
subscribe(subscriber, metadata, this);
});
Expand Down