feat: an empty minify list, and deleting an original alone (#743)

* feat: deleting an original takes its own file and nothing else

The asset it read goes alone, so a source map or another generator's file beside it stays, and a function decides per asset which of them to delete.

* feat: an empty minify list is nothing to minify

For an instance whose whole job is its generate. A list with none in it needs no guard where a second kind of value would, which is why false stays out.

* fix: nothing minifying salts no hash, and names no file beside itself

An instance whose whole job is its generate rewrote no bytes, so the bundle kept its own name; and a file written under the original's name has replaced it, so recording related there wrote the source back over it.

* feat: a generator names its file with a function too, and minify is typed as written

The public type said one function where the schema has taken a list or a descriptor since 5.10, and a filename function is what webpack's own getPath takes.

* docs: compressing with minifying, and without

The two shapes a compression instance is written in, now that an empty list of minimizers says the second one.

* fix: nothing minifying salts no embedded-source hash either

The tap varies a module's hash on the minimizers, so an instance with none of them renamed files it never rewrote — the same defect the chunk hash had.

* fix: delete the asset that was read, not whatever holds its name

The generators of a pass run together, so another may have written over the original first — deleting then destroys that file instead.

Author: alexander-akait

.changeset/generator-compression-options.md

@@ -2,4 +2,4 @@
 "minimizer-webpack-plugin": minor
 ---
 
-Add `threshold`, `minRatio` and `relatedName` to an `asset` generator, and stop `minify` from taking `false`.
+Add `threshold`, `minRatio` and `relatedName` to an `asset` generator, let `deleteOriginalAssets` be a function and take the original file alone, take a `filename` function, read `minify: []` as nothing to minify rather than `false`, and type what `minify` has long accepted.

README.md

@@ -340,13 +340,36 @@ interface minimizer {
 }
 
 type minify = minifyFn | (minifyFn | minimizer)[] | minimizer;
+// An empty array is nothing to minify.
 ```
 
 Default: `MinimizerPlugin.terserMinify`
 
 Which minimizer runs, and the options it runs with. By default the plugin uses
 [terser](https://github.com/terser/terser); overriding it is also how you test
-an unpublished version or a fork.
+an unpublished version or a fork. The default stands whether or not a
+[`generate`](#generate) was configured too, and so does [`test`](#test)'s: the
+JavaScript minifier and the names it reads are what this plugin is.
+
+Nothing minifying is an **empty array** rather than a missing value, for an
+instance whose whole job is its `generate`:
+
+```js
+new MinimizerPlugin({
+  test: /.*/,
+  minify: [],
+  generate: {
+    implementation: MinimizerPlugin.compress,
+    options: { algorithm: "gzip" },
+    type: "asset",
+    filename: "[path][base].gz",
+  },
+});
+```
+
+`false` is not accepted. A list with nothing in it needs no guard at any of the
+places that run the minimizers — each simply does nothing — while a second kind
+of value does.
 
 > **Warning**
 >
@@ -690,9 +713,12 @@ interface generator {
   implementation: generateFn;
   options?: Record<string, any>;
   type?: "import" | "asset";
-  filename?: string;
+  filename?: string | ((pathData: any) => string);
   filter?: (name: string) => boolean;
-  deleteOriginalAssets?: boolean;
+  deleteOriginalAssets?: boolean | ((name: string) => boolean);
+  threshold?: number;
+  minRatio?: number;
+  relatedName?: string | false;
 }
 
 type generate =
@@ -793,13 +819,23 @@ new MinimizerPlugin({
       implementation: MinimizerPlugin.sharpGenerate,
       options: { encodeOptions: { webp: {} } },
       type: "asset",
-      // Optional. Without it the generator's own name for the result is used,
-      // which for `sharpGenerate` is the original with its extension replaced.
+      // Optional, a template or a function answering with one. Without it the
+      // generator's own name for the result is used, which for `sharpGenerate`
+      // is the original with its extension replaced.
       filename: "[path][name].webp",
       // Optional. Narrows what this generator reads, on top of `test`.
       filter: (name) => !name.includes("icons/"),
       // Optional, `false` by default: the asset it read stays where it is.
+      // Written as a function it is asked per asset.
       deleteOriginalAssets: false,
+      // Optional. Skips an asset this small, before the generator is asked.
+      threshold: 10240,
+      // Optional. Drops the result unless it is this much smaller than what
+      // it read, as `generated size / original size`.
+      minRatio: 0.8,
+      // Optional. The key the new file is recorded under in the original's
+      // `related` info, which is how a server asked for one finds the other.
+      relatedName: "webp",
     },
   },
 });
@@ -808,14 +844,14 @@ new MinimizerPlugin({
 `type` decides which of the two things a generator does, and they are not
 interchangeable — they read different input, at different points in the build:
 
-|                                | `"import"` (the default)                   | `"asset"`                                               |
-| :----------------------------- | :----------------------------------------- | :------------------------------------------------------ |
-| Reads                          | a module, **as it builds**                 | an asset, **once it is emitted**                        |
-| Produces                       | that module's own bytes, renamed with them | a **new file beside** the one it read                   |
-| Picked by                      | `?as=<name>` on the import                 | `test` / `include` / `exclude`, then `filter`           |
-| Reaches a file nothing imports | no                                         | yes — copied assets included                            |
-| Fields it reads                | `implementation`, `options`                | those plus `filename`, `filter`, `deleteOriginalAssets` |
-| webpack                        | **5.111** or newer                         | any supported version                                   |
+|                                | `"import"` (the default)                   | `"asset"`                                                                                       |
+| :----------------------------- | :----------------------------------------- | :---------------------------------------------------------------------------------------------- |
+| Reads                          | a module, **as it builds**                 | an asset, **once it is emitted**                                                                |
+| Produces                       | that module's own bytes, renamed with them | a **new file beside** the one it read                                                           |
+| Picked by                      | `?as=<name>` on the import                 | `test` / `include` / `exclude`, then `filter`                                                   |
+| Reaches a file nothing imports | no                                         | yes — copied assets included                                                                    |
+| Fields it reads                | `implementation`, `options`                | those plus `filename`, `filter`, `deleteOriginalAssets`, `threshold`, `minRatio`, `relatedName` |
+| webpack                        | **5.111** or newer                         | any supported version                                                                           |
 
 **`"import"`** is the only point at which a rename can reach the bundle: the
 asset is named while its module is built, so every reference follows it. The
@@ -873,9 +909,35 @@ photo.jpg     still there, unless `deleteOriginalAssets`
 photo.webp    generated beside it
 ```
 
-`filename`, `filter` and `deleteOriginalAssets` describe a file being written
-beside another, so they belong to `"asset"` and setting one on an `"import"`
-generator is an error rather than a field that quietly does nothing.
+`filename`, `filter`, `deleteOriginalAssets`, `threshold`, `minRatio` and
+`relatedName` describe a file being written beside another, so they belong to
+`"asset"` and setting one on an `"import"` generator is an error rather than a
+field that quietly does nothing.
+
+Three of them decide whether the new file is worth having. `threshold` skips an
+asset too small to bother with, before the generator is asked at all. `minRatio`
+drops a result that is not enough smaller than what it read, since a file that
+saves nothing still costs a request. `relatedName` records the new file under
+that key in the original's `related` info — which is how a server asked for the
+original finds it — and declines an asset already carrying that key.
+
+A file written under the original's own name has replaced it, so there is
+nothing beside it to delete and nothing for `relatedName` to point at; a
+generator doing that re-encodes an asset in place.
+
+Deleting takes the original file and nothing else. webpack deletes whatever an
+asset's `related` names along with it, so a source map, or the file a second
+generator wrote beside the same original, would go too; the original goes
+alone instead. Where the generated file took the original's own name there is
+nothing left to delete, and `relatedName` is not recorded when the asset that
+would carry it is being deleted.
+
+A generated file inherits nothing from the one it was read from: what the
+original's info says about its hashes, its module and where its source came
+from is true of that file and not of this one. The exception is `immutable`,
+and only where `filename` still derives from the original's name — `[name]`,
+`[base]` or `[file]` — since that is what carried the hash the promise rests
+on.
 
 **When** a generator runs is not among them, because it is not the config's to
 say: the implementation declares it through a `getStage` of its own, the way it
@@ -934,6 +996,46 @@ what `stage` states rather than what applying two plugins in the right order
 happens to give. Each algorithm is run at its own maximum by default (`zlib`'s
 best level, brotli's best quality); name `compressionOptions` to say otherwise.
 
+#### Compressing with minifying, and without
+
+The example above does both: `minify` defaults to terser, so the bundle is
+minified and the compressed files are written from what minification produced.
+That is the shape to want — one pass of filtering, one cache, and `stage`
+ordering the two — and it needs nothing said about `minify` at all:
+
+```js
+new MinimizerPlugin({
+  test: /\.(js|css|html|svg)$/i,
+  generate: {
+    implementation: MinimizerPlugin.compress,
+    options: { algorithm: "gzip" },
+    type: "asset",
+    filename: "[path][base].gz",
+  },
+});
+```
+
+Compressing **only** — an instance that must not touch what it reads — says so
+with an empty list of minimizers. `test` is then yours to state too, since the
+`.js` default belongs to minifying:
+
+```js
+new MinimizerPlugin({
+  test: /.*/,
+  minify: [],
+  generate: {
+    implementation: MinimizerPlugin.compress,
+    options: { algorithm: "gzip" },
+    type: "asset",
+    filename: "[path][base].gz",
+  },
+});
+```
+
+Nothing is minified, no asset is marked `minimized`, and the bundle keeps the
+name it would have had without this plugin — an instance that rewrites nothing
+salts no hash.
+
 It is an ordinary minimizer too, so [`minify`](#minify) takes it the way it
 takes `terserMinify` or `swcMinify`. There it compresses the asset **in place**
 rather than beside it — the shape for a server that says what the encoding is

src/index.js

@@ -211,9 +211,9 @@ const {
  * @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 {(string | ((pathData: EXPECTED_ANY) => string))=} filename name for the generated asset, as a webpack filename template or a function answering with one. `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 {(boolean | ((name: string) => boolean))=} deleteOriginalAssets removes the asset generated from, its own file alone — whatever its `related` names stays. Written as a function it is asked per asset. `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
@@ -237,8 +237,24 @@ const {
  */
 
 /**
+ * One minimizer, written as an object stating how to run it.
  * @template T
- * @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
+ * @typedef {object} MinimizerDescriptor
+ * @property {MinimizerImplementation<T>} implementation the minimizer itself
+ * @property {MinimizerOptions<T>=} options options for this minimizer, preferred over the deprecated `minimizerOptions`
+ * @property {((name: string, info: AssetInfo) => boolean | undefined)=} filter which assets this minimizer is offered, overriding a `filter` on the function itself
+ */
+
+/**
+ * What `minify` may be written as: one minimizer, a list of them — empty for
+ * nothing to minify — or a descriptor.
+ * @template T
+ * @typedef {MinimizerImplementation<T> | (MinimizerImplementation<T> | MinimizerDescriptor<T>)[] | MinimizerDescriptor<T>} Minify
+ */
+
+/**
+ * @template T
+ * @typedef {T extends import("terser").MinifyOptions ? { minify?: Minify<T> | undefined, minimizerOptions?: MinimizerOptions<T> | undefined, terserOptions?: MinimizerOptions<T> | undefined } : { minify: Minify<T>, minimizerOptions?: MinimizerOptions<T> | undefined, terserOptions?: MinimizerOptions<T> | undefined }} DefinedDefaultMinimizerAndOptions
  */
 
 /**
@@ -1355,7 +1371,7 @@ class MinimizerPlugin {
    * @param {string | undefined} name the preset it is written under, where it has one
    * @param {EXPECTED_ANY} entry what was written there
    * @param {EXPECTED_ANY} declared what `generatorOptions` says for it
-   * @returns {{ name: string | undefined, implementation: EXPECTED_ANY, options: EXPECTED_ANY, type: string | undefined, filename: string | undefined, filter: ((name: string) => boolean) | undefined, deleteOriginalAssets: boolean | undefined, threshold: number | undefined, minRatio: number | undefined, relatedName: string | false | undefined }} the generator
+   * @returns {{ name: string | undefined, implementation: EXPECTED_ANY, options: EXPECTED_ANY, type: string | undefined, filename: string | ((pathData: EXPECTED_ANY) => string) | undefined, filter: ((name: string) => boolean) | undefined, deleteOriginalAssets: boolean | ((name: string) => boolean) | undefined, threshold: number | undefined, minRatio: number | undefined, relatedName: string | false | undefined }} the generator
    */
   describeGenerator(name, entry, declared) {
     const descriptor = isDescriptor(entry) ? entry : undefined;
@@ -1758,21 +1774,37 @@ class MinimizerPlugin {
       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.
+    const deletes =
+      typeof generator.deleteOriginalAssets === "function"
+        ? generator.deleteOriginalAssets(name)
+        : generator.deleteOriginalAssets;
+
+    if (deletes) {
+      const still = compilation.getAsset(name);
+
       // A generator writing under the original's own name leaves nothing to
-      // delete either: that file is now the generated one.
-      if (generatedName !== name && compilation.getAsset(name)) {
+      // delete: that file is now the generated one. Another generator of this
+      // pass may have written over it too, which is the same thing — what the
+      // name holds is no longer what this one read.
+      if (generatedName !== name && still && still.source === source) {
+        // Deleting an asset takes everything its `related` names with it — a
+        // source map, another generator's file — so it goes alone.
+        compilation.updateAsset(name, source, (was) => {
+          const { related, ...rest } = was || {};
+
+          return rest;
+        });
         compilation.deleteAsset(name);
       }
 
       return;
     }
 
     // Recorded on the asset it was read from, which is how a server asked for
-    // that one finds this one.
-    if (generator.relatedName) {
+    // that one finds this one. A file written under that same name has replaced
+    // it, so there is nothing left to point anywhere, and writing the source
+    // back would undo what was just generated.
+    if (generator.relatedName && generatedName !== name) {
       compilation.updateAsset(name, source, {
         related: { [generator.relatedName]: generatedName },
       });
@@ -2156,7 +2188,9 @@ class MinimizerPlugin {
     const written = Array.isArray(minify) ? minify : [minify];
 
     for (const [index, one] of written.entries()) {
-      const own = isDescriptor(one) ? one.options : undefined;
+      const own = isDescriptor(one)
+        ? /** @type {MinimizerDescriptor<EXPECTED_ANY>} */ (one).options
+        : undefined;
       const twice = Array.isArray(minify)
         ? getMinimizerOptionsAt(declared, index)
         : declared;
@@ -2355,6 +2389,12 @@ class MinimizerPlugin {
       });
 
       hooks.chunkHash.tap(pluginName, (chunk, hash) => {
+        // Nothing minifying rewrites nothing, so no name owes it a hash of its
+        // own.
+        if (this.minimizers().length === 0) {
+          return;
+        }
+
         const willBe = chunkAssetName(compilation, chunk);
 
         // A chunk this instance was never pointed at cannot vary with its
@@ -2377,7 +2417,10 @@ class MinimizerPlugin {
         /** @type {EmbeddedSourceHooks} */
         (/** @type {unknown} */ (compilation.hooks));
 
+      // Nothing minifying rewrites no embedded source either, and salting the
+      // module hash would rename a file this instance never touches.
       if (
+        this.minimizers().length > 0 &&
         embeddedHooks.renderEmbeddedSource &&
         embeddedHooks.embeddedSourceHash
       ) {