Fix whitespace and simplify unwrapping logic per review feedback

- Remove whitespace from MODULE_WRAPPER_SHORTHAND_PREFIX/SUFFIX (minifier strips it)
- Change moduleShorthandFormat from Map to Set (only true values matter)
- Simplify unwrapping logic to just use prefix/suffix lengths instead of complex searching
- All tests passing

Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>

Author: Copilot

common/reviews/api/webpack5-module-minifier-plugin.api.md

@@ -111,10 +111,10 @@ export interface IRenderedModulePosition {
 export const MODULE_WRAPPER_PREFIX: '__MINIFY_MODULE__(';
 
 // @public
-export const MODULE_WRAPPER_SHORTHAND_PREFIX: '__MINIFY_MODULE__({\n__DEFAULT_ID__';
+export const MODULE_WRAPPER_SHORTHAND_PREFIX: '__MINIFY_MODULE__({__DEFAULT_ID__';
 
 // @public
-export const MODULE_WRAPPER_SHORTHAND_SUFFIX: '\n});';
+export const MODULE_WRAPPER_SHORTHAND_SUFFIX: '});';
 
 // @public
 export const MODULE_WRAPPER_SUFFIX: ');';

webpack/webpack5-module-minifier-plugin/src/Constants.ts

@@ -21,16 +21,16 @@ export const MODULE_WRAPPER_SUFFIX: ');' = ');';
  * Public because alternate Minifier implementations may wish to know about it.
  * @public
  */
-export const MODULE_WRAPPER_SHORTHAND_PREFIX: '__MINIFY_MODULE__({\n__DEFAULT_ID__' =
-  '__MINIFY_MODULE__({\n__DEFAULT_ID__';
+export const MODULE_WRAPPER_SHORTHAND_PREFIX: '__MINIFY_MODULE__({__DEFAULT_ID__' =
+  '__MINIFY_MODULE__({__DEFAULT_ID__';
 /**
  * Suffix to wrap ECMAScript method shorthand `(module, __webpack_exports__, __webpack_require__) { ... }` so that the minifier doesn't delete it.
  * Used when webpack emits modules using shorthand syntax.
  * Combined with the prefix, creates: `__MINIFY_MODULE__({__DEFAULT_ID__(params){body}});`
  * Public because alternate Minifier implementations may wish to know about it.
  * @public
  */
-export const MODULE_WRAPPER_SHORTHAND_SUFFIX: '\n});' = '\n});';
+export const MODULE_WRAPPER_SHORTHAND_SUFFIX: '});' = '});';
 
 /**
  * Token preceding a module id in the emitted asset so the minifier can operate on the Webpack runtime or chunk boilerplate in isolation

webpack/webpack5-module-minifier-plugin/src/ModuleMinifierPlugin.ts

@@ -258,10 +258,9 @@ export class ModuleMinifierPlugin implements WebpackPluginInstance {
       const submittedModules: Set<string | number> = new Set();
 
       /**
-       * Map to track which modules use ECMAScript method shorthand format.
-       * Key is the module hash, value is true if shorthand format is used.
+       * Set of module hashes that use ECMAScript method shorthand format.
        */
-      const moduleShorthandFormat: Map<string, boolean> = new Map();
+      const moduleShorthandFormat: Set<string> = new Set();
 
       /**
        * The text and comments of all minified modules.
@@ -403,7 +402,9 @@ export class ModuleMinifierPlugin implements WebpackPluginInstance {
             submittedModules.add(hash);
 
             // Track whether this module uses shorthand format
-            moduleShorthandFormat.set(hash, isShorthand);
+            if (isShorthand) {
+              moduleShorthandFormat.add(hash);
+            }
 
             ++pendingMinificationRequests;
 
@@ -436,39 +437,15 @@ export class ModuleMinifierPlugin implements WebpackPluginInstance {
                     const len: number = minified.length;
 
                     // Trim off the boilerplate used to preserve the factory
-                    // Use different logic for shorthand vs regular format
-                    const isShorthandModule: boolean = moduleShorthandFormat.get(hash) || false;
+                    // Use different prefix/suffix lengths for shorthand vs regular format
+                    const isShorthandModule: boolean = moduleShorthandFormat.has(hash);
                     if (isShorthandModule) {
-                      // For shorthand format, we wrapped it as: __MINIFY_MODULE__({\n__DEFAULT_ID__(args) {...}\n});
-                      // After minification, it becomes: __MINIFY_MODULE__({__DEFAULT_ID__(args){...}});
-                      // We need to extract just: (args){...}
-
-                      // Strategy: Find and remove the prefix up to and including '__DEFAULT_ID__'
-                      const defaultIdStr: string = '__DEFAULT_ID__';
-                      const shorthandPrefixEnd: number = minified.indexOf(defaultIdStr);
-                      if (shorthandPrefixEnd >= 0) {
-                        // Remove from start up to and including '__DEFAULT_ID__'
-                        unwrapped.replace(0, shorthandPrefixEnd + defaultIdStr.length - 1, '');
-                        // Remove the suffix from the end
-                        // After minification, the suffix is typically '});' (3 characters, newline removed)
-                        const minifiedSuffix: string = '});';
-                        if (minified.endsWith(minifiedSuffix)) {
-                          unwrapped.replace(len - minifiedSuffix.length, len - 1, '');
-                        } else if (minified.endsWith(MODULE_WRAPPER_SHORTHAND_SUFFIX)) {
-                          // In case minifier keeps the newline
-                          unwrapped.replace(len - MODULE_WRAPPER_SHORTHAND_SUFFIX.length, len - 1, '');
-                        } else {
-                          // Fallback: assume '});' suffix
-                          unwrapped.replace(len - minifiedSuffix.length, len - 1, '');
-                        }
-                      } else {
-                        // Fallback: If __DEFAULT_ID__ is not found (shouldn't happen), remove by length
-                        const minifiedSuffix: string = '});';
-                        unwrapped.replace(0, MODULE_WRAPPER_SHORTHAND_PREFIX.length - 1, '');
-                        unwrapped.replace(len - minifiedSuffix.length, len - 1, '');
-                      }
+                      // For shorthand format: __MINIFY_MODULE__({__DEFAULT_ID__(args){...}});
+                      // Remove prefix and suffix by their lengths
+                      unwrapped.replace(0, MODULE_WRAPPER_SHORTHAND_PREFIX.length - 1, '');
+                      unwrapped.replace(len - MODULE_WRAPPER_SHORTHAND_SUFFIX.length, len - 1, '');
                     } else {
-                      // Regular format
+                      // Regular format: __MINIFY_MODULE__((args){...});
                       unwrapped.replace(0, MODULE_WRAPPER_PREFIX.length - 1, '');
                       unwrapped.replace(len - MODULE_WRAPPER_SUFFIX.length, len - 1, '');
                     }

webpack/webpack5-module-minifier-plugin/src/test/__snapshots__/AmdExternals.test.ts.snap

@@ -3,7 +3,7 @@
 exports[`ModuleMinifierPlugin Handles AMD externals (mock): Content 1`] = `
 Object {
   "/release/async.js": "/*! For license information please see async.js.LICENSE.txt */
-// Begin Asset Hash=68ab9ab7e0ab87d08307091bb1e31e56e04b12fc882535d275e7d4c9402a5d8b
+// Begin Asset Hash=cfb7f7d353fd9d01c51f613f9235ad90b50fe78f746b088cf52a92517a5fc458
 \\"use strict\\";
 (self[\\"webpackChunk\\"] = self[\\"webpackChunk\\"] || []).push([[157],{
 
@@ -344,7 +344,7 @@ exports[`ModuleMinifierPlugin Handles AMD externals (terser): Content 1`] = `Obj
 exports[`ModuleMinifierPlugin Handles AMD externals (terser): Errors 1`] = `
 Array [
   Object {
-    "message": "Unexpected token name «__WEBPACK_CHUNK_MODULE__54d801f652de2ab1c87a646d9bc448cff4235f0d68b5519fdb388f0c0c0af447», expected punc «,»",
+    "message": "Unexpected token name «__WEBPACK_CHUNK_MODULE__aa928190bac95603578bbc24c65647633e6305dc69d46edaf07d710fc69da78d», expected punc «,»",
   },
 ]
 `;

webpack/webpack5-module-minifier-plugin/src/test/__snapshots__/MultipleRuntimes.test.ts.snap

@@ -3,7 +3,7 @@
 exports[`ModuleMinifierPlugin Handles multiple runtimes (mock): Content 1`] = `
 Object {
   "/release/async-1.js": "/*! For license information please see async-1.js.LICENSE.txt */
-// Begin Asset Hash=69b04c1d29fa46faf36d042bbe433bfe80bf6de89433dda1e91248932b0bdaaf
+// Begin Asset Hash=b53e0a088f83c5d7fc7ba3ca806e0471adea1460f448a26004ed99835ccffac3
 \\"use strict\\";
 (self[\\"webpackChunk\\"] = self[\\"webpackChunk\\"] || []).push([[527],{
 
@@ -39,7 +39,7 @@ function async1() { console.log('async-1'); }
 // @license MIT
 ",
   "/release/async-2.js": "/*! For license information please see async-2.js.LICENSE.txt */
-// Begin Asset Hash=d8c163257b9c0ffa0929f47ce896d7edbb1412dc70ce4327c3fc9e2d7a028e92
+// Begin Asset Hash=2e943d5dc5a407a894b565d54ceeb3f554742f52442d698c22b8795be4d4694c
 \\"use strict\\";
 (self[\\"webpackChunk\\"] = self[\\"webpackChunk\\"] || []).push([[324],{
 
@@ -635,10 +635,10 @@ exports[`ModuleMinifierPlugin Handles multiple runtimes (terser): Content 1`] =
 exports[`ModuleMinifierPlugin Handles multiple runtimes (terser): Errors 1`] = `
 Array [
   Object {
-    "message": "Unexpected token name «__WEBPACK_CHUNK_MODULE__c3587d3a82556f8e3f1b0b4d1fcbdefd62eedc2f1c5708b0a824a504bef85276», expected punc «,»",
+    "message": "Unexpected token name «__WEBPACK_CHUNK_MODULE__522ee27cac2edf706f26b73b1e5ec4bc44feefe1107c5e303f671547dff3b8d1», expected punc «,»",
   },
   Object {
-    "message": "Unexpected token name «__WEBPACK_CHUNK_MODULE__bf7c8493af96fc819aeda647ea3306127d1b5c0239a14cb225810a4644dfe910», expected punc «,»",
+    "message": "Unexpected token name «__WEBPACK_CHUNK_MODULE__9fa9db01a39f213b0bad42889493581d8a3cc9d76f3b77543aa34e2cfc6622ed», expected punc «,»",
   },
 ]
 `;