fix: four things an automated review was right about

Deleting an asset takes everything its `related` names with it, so recording the generated file there and then deleting the original deleted the file just written — `relatedName` and `deleteOriginalAssets` together emitted nothing at all. The one is now recorded only where the other leaves the original standing, which is what `compression-webpack-plugin` has always done.

A generated name that already existed was written over and then returned from, before either of those was reached, so a rebuild owed what a first build paid. The chunk-hash guard asked a `filter` for its answer while handing it an empty object for the asset info it reads, which could decline a chunk it then took: only `test`/`include`/`exclude` decide there, being the part that reads a name.

And the descriptor an `asset` generator is written as had no type, so none of `type`, `filename`, `threshold`, `minRatio` or `relatedName` could be reached from TypeScript at all; `minify` no longer offers the `false` the schema rejects.

Author: alexander-akait

src/index.js

@@ -205,20 +205,40 @@ const {
  * @typedef {undefined | boolean | number} Parallel
  */
 
+/**
+ * One generator, written as an object stating how to run it.
+ * @typedef {object} GeneratorDescriptor
+ * @property {MinimizerImplementation<EXPECTED_ANY>} implementation the generator itself
+ * @property {MinimizerOptions<EXPECTED_ANY>=} options options for this generator, preferred over the deprecated `generatorOptions`
+ * @property {("import" | "asset")=} type `import` re-encodes a module as it is built, so the import that asked for it is renamed with it; `asset` writes a new file beside one already emitted
+ * @property {string=} filename name for the generated asset, as a webpack filename template. `asset` generators only
+ * @property {((name: string) => boolean)=} filter decides per asset whether to generate from it, on top of `test`/`include`/`exclude`
+ * @property {boolean=} deleteOriginalAssets removes the asset generated from. `asset` generators only
+ * @property {number=} threshold generate only from assets larger than this, in bytes. `asset` generators only
+ * @property {number=} minRatio keep the generated asset only when it is this much smaller than the one it was read from. `asset` generators only
+ * @property {(string | false)=} relatedName the key the generated asset is recorded under in the original's `related` info. `asset` generators only
+ */
+
+/**
+ * What `generate` may be written as: one generator, a list of them, a
+ * descriptor, or an object naming descriptors an asset asks for with `?as=`.
+ * @typedef {MinimizerImplementation<EXPECTED_ANY> | MinimizerImplementation<EXPECTED_ANY>[] | GeneratorDescriptor | { [preset: string]: MinimizerImplementation<EXPECTED_ANY> | MinimizerImplementation<EXPECTED_ANY>[] | GeneratorDescriptor }} Generate
+ */
+
 /**
  * @typedef {object} BasePluginOptions
  * @property {Rules=} test test rule
  * @property {Rules=} include include rile
  * @property {Rules=} exclude exclude rule
  * @property {ExtractCommentsOptions=} extractComments extract comments options
  * @property {Parallel=} parallel parallel option
- * @property {MinimizerImplementation<EXPECTED_ANY>=} generate rewrites a module's own bytes as it is built, so a re-encoding can rename the asset
+ * @property {Generate=} generate rewrites a module's own bytes as it is built, so a re-encoding can rename the asset, or writes a new file beside one already emitted
  * @property {MinimizerOptions<EXPECTED_ANY>=} generatorOptions options for `generate`
  */
 
 /**
  * @template T
- * @typedef {T extends import("terser").MinifyOptions ? { minify?: MinimizerImplementation<T> | false | undefined, minimizerOptions?: MinimizerOptions<T> | undefined, terserOptions?: MinimizerOptions<T> | undefined } : { minify: MinimizerImplementation<T> | false, minimizerOptions?: MinimizerOptions<T> | undefined, terserOptions?: MinimizerOptions<T> | undefined }} DefinedDefaultMinimizerAndOptions
+ * @typedef {T extends import("terser").MinifyOptions ? { minify?: MinimizerImplementation<T> | undefined, minimizerOptions?: MinimizerOptions<T> | undefined, terserOptions?: MinimizerOptions<T> | undefined } : { minify: MinimizerImplementation<T>, minimizerOptions?: MinimizerOptions<T> | undefined, terserOptions?: MinimizerOptions<T> | undefined }} DefinedDefaultMinimizerAndOptions
  */
 
 /**
@@ -590,31 +610,6 @@ class MinimizerPlugin {
     return !(exclude && (matchPart(name, exclude) || matchPart(bare, exclude)));
   }
 
-  /**
-   * Whether any configured minimizer would be handed an asset of this name,
-   * by the plugin's own `test`/`include`/`exclude` and then by its own filter.
-   * @private
-   * @param {Compiler} compiler compiler
-   * @param {string} name asset name
-   * @returns {boolean} true when one of them would take it
-   */
-  minifiesName(compiler, name) {
-    if (!this.matchesName(compiler, name)) {
-      return false;
-    }
-
-    const { filters } = this.options.minimizer;
-
-    return this.minimizers().some((implementation, i) => {
-      const decides =
-        filters && typeof filters[i] === "function"
-          ? filters[i]
-          : implementation.filter;
-
-      return typeof decides !== "function" || decides(name, {}) !== false;
-    });
-  }
-
   /**
    * @private
    * @param {Compiler} compiler compiler
@@ -1720,25 +1715,31 @@ class MinimizerPlugin {
       generatedInfo.immutable = true;
     }
 
+    // A rebuild writes over the file it wrote last time rather than a new one,
+    // and what is recorded below is owed either way.
     if (compilation.getAsset(generatedName)) {
       compilation.updateAsset(generatedName, generatedSource, generatedInfo);
+    } else {
+      compilation.emitAsset(generatedName, generatedSource, generatedInfo);
+    }
+
+    if (generator.deleteOriginalAssets) {
+      // Deleting an asset takes everything its `related` names with it, so
+      // recording this file there first would delete the file just written.
+      if (compilation.getAsset(name)) {
+        compilation.deleteAsset(name);
+      }
 
       return;
     }
 
-    compilation.emitAsset(generatedName, generatedSource, generatedInfo);
-
     // Recorded on the asset it was read from, which is how a server asked for
     // that one finds this one.
     if (generator.relatedName) {
       compilation.updateAsset(name, source, {
         related: { [generator.relatedName]: generatedName },
       });
     }
-
-    if (generator.deleteOriginalAssets && compilation.getAsset(name)) {
-      compilation.deleteAsset(name);
-    }
   }
 
   /**
@@ -2328,12 +2329,11 @@ class MinimizerPlugin {
       hooks.chunkHash.tap(pluginName, (chunk, hash) => {
         const willBe = chunkAssetName(compilation, chunk);
 
-        // A chunk no minimizer here would be handed cannot vary with them, so
-        // salting it would rename a file this instance never rewrites.
-        if (
-          typeof willBe === "string" &&
-          !this.minifiesName(compiler, willBe)
-        ) {
+        // A chunk this instance was never pointed at cannot vary with its
+        // minimizers, so salting it would rename a file it never rewrites. A
+        // `filter` is not asked: it reads an asset's info, which no asset has
+        // yet, and guessing one could skip the salt for an asset it then takes.
+        if (typeof willBe === "string" && !this.matchesName(compiler, willBe)) {
           return;
         }
 

test/generate-option.test.js

@@ -2119,6 +2119,183 @@ describe("generate beside the minifier", () => {
   });
 });
 
+describe("generate over a file that is already there", () => {
+  it("should record `related` and delete the original even when the file exists", async () => {
+    const compiler = getCompiler({
+      entry: path.resolve(__dirname, "./fixtures/images.js"),
+      module: { rules: IMAGE_RULES },
+    });
+
+    /** Writes the name the generator is about to write, before it runs. */
+    class AlreadyThere {
+      /**
+       * @param {import("webpack").Compiler} instance compiler
+       * @returns {void}
+       */
+      apply(instance) {
+        instance.hooks.compilation.tap("AlreadyThere", (compilation) => {
+          compilation.hooks.processAssets.tap(
+            {
+              name: "AlreadyThere",
+              stage:
+                compiler.webpack.Compilation.PROCESS_ASSETS_STAGE_ADDITIONAL,
+            },
+            () => {
+              compilation.emitAsset(
+                "image.copy.png",
+                new compiler.webpack.sources.RawSource(Buffer.from("stale")),
+              );
+            },
+          );
+        });
+      }
+    }
+
+    new AlreadyThere().apply(compiler);
+    new MinimizerPlugin({
+      test: /^image\.png$/i,
+      generate: {
+        implementation: (input) => ({
+          code: Buffer.from(Object.values(input)[0]),
+        }),
+        type: "asset",
+        filename: "[path][name].copy[ext]",
+        relatedName: "copied",
+        deleteOriginalAssets: true,
+      },
+    }).apply(compiler);
+
+    const stats = await compile(compiler);
+    const names = Object.keys(stats.compilation.assets);
+
+    expect(getErrors(stats)).toEqual([]);
+    // Written over rather than emitted, and the original still gone with it.
+    expect(names).toContain("image.copy.png");
+    expect(names).not.toContain("image.png");
+    expect(readAsset("image.copy.png", compiler, stats)).not.toBe("stale");
+  });
+
+  it("should record `related` on the original where it is kept", async () => {
+    const compiler = getCompiler({
+      entry: path.resolve(__dirname, "./fixtures/images.js"),
+      module: { rules: IMAGE_RULES },
+    });
+
+    /** Writes the name the generator is about to write, before it runs. */
+    class AlreadyThere {
+      /**
+       * @param {import("webpack").Compiler} instance compiler
+       * @returns {void}
+       */
+      apply(instance) {
+        instance.hooks.compilation.tap("AlreadyThere", (compilation) => {
+          compilation.hooks.processAssets.tap(
+            {
+              name: "AlreadyThere",
+              stage:
+                compiler.webpack.Compilation.PROCESS_ASSETS_STAGE_ADDITIONAL,
+            },
+            () => {
+              compilation.emitAsset(
+                "image.copy.png",
+                new compiler.webpack.sources.RawSource(Buffer.from("stale")),
+              );
+            },
+          );
+        });
+      }
+    }
+
+    new AlreadyThere().apply(compiler);
+    new MinimizerPlugin({
+      test: /^image\.png$/i,
+      generate: {
+        implementation: (input) => ({
+          code: Buffer.from(Object.values(input)[0]),
+        }),
+        type: "asset",
+        filename: "[path][name].copy[ext]",
+        relatedName: "copied",
+      },
+    }).apply(compiler);
+
+    const stats = await compile(compiler);
+    const original = /** @type {import("webpack").Asset} */ (
+      stats.compilation.getAsset("image.png")
+    );
+
+    expect(getErrors(stats)).toEqual([]);
+    expect(
+      /** @type {{ [key: string]: string }} */ (original.info.related).copied,
+    ).toBe("image.copy.png");
+  });
+});
+
+describe("deleting the asset a file was written beside", () => {
+  it("should keep the generated file when `relatedName` is set too", async () => {
+    const compiler = getCompiler({
+      entry: path.resolve(__dirname, "./fixtures/images.js"),
+      module: { rules: IMAGE_RULES },
+    });
+
+    new MinimizerPlugin({
+      test: /^image\.png$/i,
+      generate: {
+        implementation: (input) => ({
+          code: Buffer.from(Object.values(input)[0]),
+        }),
+        type: "asset",
+        filename: "[path][name].copy[ext]",
+        relatedName: "copied",
+        deleteOriginalAssets: true,
+      },
+    }).apply(compiler);
+
+    const stats = await compile(compiler);
+    const names = Object.keys(stats.compilation.assets);
+
+    // Deleting an asset takes everything its `related` names with it, so the
+    // two together must not delete the file that was just written.
+    expect(getErrors(stats)).toEqual([]);
+    expect(names).toContain("image.copy.png");
+    expect(names).not.toContain("image.png");
+  });
+
+  it("should not mind a second generator having deleted it already", async () => {
+    const compiler = getCompiler({
+      entry: path.resolve(__dirname, "./fixtures/images.js"),
+      module: { rules: IMAGE_RULES },
+    });
+
+    /**
+     * @param {string} suffix what to name what it writes
+     * @returns {EXPECTED_ANY} one generator
+     */
+    const copyTo = (suffix) => ({
+      implementation: (input) => ({
+        code: Buffer.from(Object.values(input)[0]),
+      }),
+      type: "asset",
+      filename: `[path][name].${suffix}[ext]`,
+      deleteOriginalAssets: true,
+    });
+
+    new MinimizerPlugin({
+      test: /^image\.png$/i,
+      generate: { one: copyTo("one"), two: copyTo("two") },
+    }).apply(compiler);
+
+    const stats = await compile(compiler);
+    const names = Object.keys(stats.compilation.assets);
+
+    // Both wrote, and whichever deleted second found nothing left to delete.
+    expect(getErrors(stats)).toEqual([]);
+    expect(names).toContain("image.one.png");
+    expect(names).toContain("image.two.png");
+    expect(names).not.toContain("image.png");
+  });
+});
+
 describe("generate from an asset emitted late", () => {
   it("should generate from an asset added after the generators ran", async () => {
     const seen = [];