Support cache with external dependencies (#1033)

JLHwung · liuxingbaoyu · web-flow · commit d4181b8e20c2 · 2024-09-02T11:14:04.000-04:00
* Add tests from #984 # Conflicts: # .yarnrc.yml # src/index.js * migrate test to node test styles * feat: enable cache where there are external deps * chore: fix dead links in comments * fix lint errors * save dep and timestamp as tuple * simplify handleExternalDependencies interface * chore: create getFileTimestamp only when cache is enabled --------- Co-authored-by: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com>
diff --git a/.yarnrc.yml b/.yarnrc.yml
@@ -1,2 +1,3 @@
+enableGlobalCache: true
 nodeLinker: node-modules
 yarnPath: .yarn/releases/yarn-3.6.4.cjs
diff --git a/src/cache.js b/src/cache.js
@@ -64,6 +64,37 @@ const filename = function (source, identifier, options, hash) {
   return hash.digest("hex") + ".json";
 };
 
+const addTimestamps = async function (externalDependencies, getFileTimestamp) {
+  for (const depAndEmptyTimestamp of externalDependencies) {
+    try {
+      const [dep] = depAndEmptyTimestamp;
+      const { timestamp } = await getFileTimestamp(dep);
+      depAndEmptyTimestamp.push(timestamp);
+    } catch {
+      // ignore errors if timestamp is not available
+    }
+  }
+};
+
+const areExternalDependenciesModified = async function (
+  externalDepsWithTimestamp,
+  getFileTimestamp,
+) {
+  for (const depAndTimestamp of externalDepsWithTimestamp) {
+    const [dep, timestamp] = depAndTimestamp;
+    let newTimestamp;
+    try {
+      newTimestamp = (await getFileTimestamp(dep)).timestamp;
+    } catch {
+      return true;
+    }
+    if (timestamp !== newTimestamp) {
+      return true;
+    }
+  }
+  return false;
+};
+
 /**
  * Handle the cache
  *
@@ -78,6 +109,7 @@ const handleCache = async function (directory, params) {
     cacheDirectory,
     cacheCompression,
     hash,
+    getFileTimestamp,
   } = params;
 
   const file = path.join(
@@ -88,7 +120,15 @@ const handleCache = async function (directory, params) {
   try {
     // No errors mean that the file was previously cached
     // we just need to return it
-    return await read(file, cacheCompression);
+    const result = await read(file, cacheCompression);
+    if (
+      !(await areExternalDependenciesModified(
+        result.externalDependencies,
+        getFileTimestamp,
+      ))
+    ) {
+      return result;
+    }
   } catch {
     // conitnue if cache can't be read
   }
@@ -111,20 +151,17 @@ const handleCache = async function (directory, params) {
   // Otherwise just transform the file
   // return it to the user asap and write it in cache
   const result = await transform(source, options);
+  await addTimestamps(result.externalDependencies, getFileTimestamp);
 
-  // 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);
     }
+
+    throw err;
   }
 
   return result;
diff --git a/src/index.js b/src/index.js
@@ -26,6 +26,7 @@ const injectCaller = require("./injectCaller");
 const schema = require("./schema");
 
 const { isAbsolute } = require("path");
+const { promisify } = require("util");
 
 function subscribe(subscriber, metadata, context) {
   if (context[subscriber]) {
@@ -176,6 +177,9 @@ async function loader(source, inputSourceMap, overrides) {
 
     let result;
     if (cacheDirectory) {
+      const getFileTimestamp = promisify((path, cb) => {
+        this._compilation.fileSystemInfo.getFileTimestamp(path, cb);
+      });
       const hash = this.utils.createHash(
         this._compilation.outputOptions.hashFunction,
       );
@@ -187,6 +191,7 @@ async function loader(source, inputSourceMap, overrides) {
         cacheIdentifier,
         cacheCompression,
         hash,
+        getFileTimestamp,
       });
     } else {
       result = await transform(source, options);
@@ -207,7 +212,9 @@ async function loader(source, inputSourceMap, overrides) {
 
       const { code, map, metadata, externalDependencies } = result;
 
-      externalDependencies?.forEach(dep => this.addDependency(dep));
+      externalDependencies?.forEach(([dep]) => {
+        this.addDependency(dep);
+      });
       metadataSubscribers.forEach(subscriber => {
         subscribe(subscriber, metadata, this);
       });
diff --git a/src/transform.js b/src/transform.js
@@ -16,7 +16,7 @@ module.exports = async function (source, options) {
 
   // We don't return the full result here because some entries are not
   // really serializable. For a full list of properties see here:
-  // https://github.com/babel/babel/blob/main/packages/babel-core/src/transformation/index.js
+  // https://github.com/babel/babel/blob/main/packages/babel-core/src/transformation/index.ts
   // For discussion on this topic see here:
   // https://github.com/babel/babel-loader/pull/629
   const { ast, code, map, metadata, sourceType, externalDependencies } = result;
@@ -32,7 +32,9 @@ module.exports = async function (source, options) {
     metadata,
     sourceType,
     // Convert it from a Set to an Array to make it JSON-serializable.
-    externalDependencies: Array.from(externalDependencies || []),
+    externalDependencies: Array.from(externalDependencies || [], dep => [
+      dep,
+    ]).sort(),
   };
 };
 
diff --git a/test/cache.test.js b/test/cache.test.js
@@ -222,7 +222,7 @@ test("should have one file per module", async () => {
   assert.deepEqual(stats.compilation.warnings, []);
 
   const files = fs.readdirSync(context.cacheDirectory);
-  assert.ok(files.length === 3);
+  assert.strictEqual(files.length, 3);
 });
 
 test("should generate a new file if the identifier changes", async () => {
@@ -276,7 +276,7 @@ test("should generate a new file if the identifier changes", async () => {
   );
 
   const files = fs.readdirSync(context.cacheDirectory);
-  assert.ok(files.length === 6);
+  assert.strictEqual(files.length, 6);
 });
 
 test("should allow to specify the .babelrc file", async () => {
@@ -331,5 +331,72 @@ test("should allow to specify the .babelrc file", async () => {
   const files = fs.readdirSync(context.cacheDirectory);
   // The two configs resolved to same Babel config because "fixtures/babelrc"
   // is { "presets": ["@babel/preset-env"] }
-  assert.ok(files.length === 1);
+  assert.strictEqual(files.length, 1);
+});
+
+test("should cache result when there are external dependencies", async () => {
+  const dep = path.join(cacheDir, "externalDependency.txt");
+
+  fs.writeFileSync(dep, "first update");
+
+  let counter = 0;
+
+  const config = Object.assign({}, globalConfig, {
+    entry: path.join(__dirname, "fixtures/constant.js"),
+    output: {
+      path: context.directory,
+    },
+    module: {
+      rules: [
+        {
+          test: /\.js$/,
+          loader: babelLoader,
+          options: {
+            babelrc: false,
+            configFile: false,
+            cacheDirectory: 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();
+                    },
+                  },
+                };
+              },
+            ],
+          },
+        },
+      ],
+    },
+  });
+
+  let stats = await webpackAsync(config);
+  assert.deepEqual(stats.compilation.warnings, []);
+  assert.deepEqual(stats.compilation.errors, []);
+
+  assert.ok(stats.compilation.fileDependencies.has(dep));
+  assert.strictEqual(counter, 1);
+
+  stats = await webpackAsync(config);
+  assert.deepEqual(stats.compilation.warnings, []);
+  assert.deepEqual(stats.compilation.errors, []);
+
+  assert.ok(stats.compilation.fileDependencies.has(dep));
+  assert.strictEqual(counter, 1);
+
+  fs.writeFileSync(dep, "second update");
+  stats = await webpackAsync(config);
+  assert.deepEqual(stats.compilation.warnings, []);
+  assert.deepEqual(stats.compilation.errors, []);
+
+  assert.ok(stats.compilation.fileDependencies.has(dep));
+  assert.strictEqual(counter, 2);
 });

Original file line number	Diff line number	Diff line change
`@@ -1,2 +1,3 @@`
	`1`	`+enableGlobalCache: true`
`1`	`2`	`nodeLinker: node-modules`
`2`	`3`	`yarnPath: .yarn/releases/yarn-3.6.4.cjs`