Skip to content

Commit 0990711

Browse files
Copilotdmichon-msft
andcommitted
Address code review feedback
- Use regex pattern /function\s*\(/ for more robust function keyword detection with whitespace variations - Replace magic number 3 with explicit minifiedSuffix variable for better maintainability - Add documentation showing complete wrapper structure in constants comments - All tests still passing, no security issues found Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>
1 parent a4fed76 commit 0990711

3 files changed

Lines changed: 19 additions & 6 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@rushstack/webpack5-module-minifier-plugin",
5+
"comment": "Add support for webpack's ECMAScript method shorthand format. The plugin now detects when modules are emitted using method shorthand syntax (without 'function' keyword or arrow syntax) and wraps them appropriately for minification.",
6+
"type": "minor"
7+
}
8+
],
9+
"packageName": "@rushstack/webpack5-module-minifier-plugin"
10+
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ export const MODULE_WRAPPER_SUFFIX: ');' = ');';
1717
/**
1818
* Prefix to wrap ECMAScript method shorthand `(module, __webpack_exports__, __webpack_require__) { ... }` so that the minifier doesn't delete it.
1919
* Used when webpack emits modules using shorthand syntax.
20+
* Combined with the suffix, creates: `__MINIFY_MODULE__({__DEFAULT_ID__(params){body}});`
2021
* Public because alternate Minifier implementations may wish to know about it.
2122
* @public
2223
*/
@@ -25,6 +26,7 @@ export const MODULE_WRAPPER_SHORTHAND_PREFIX: '__MINIFY_MODULE__({\n__DEFAULT_ID
2526
/**
2627
* Suffix to wrap ECMAScript method shorthand `(module, __webpack_exports__, __webpack_require__) { ... }` so that the minifier doesn't delete it.
2728
* Used when webpack emits modules using shorthand syntax.
29+
* Combined with the prefix, creates: `__MINIFY_MODULE__({__DEFAULT_ID__(params){body}});`
2830
* Public because alternate Minifier implementations may wish to know about it.
2931
* @public
3032
*/

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,8 @@ function isMethodShorthandFormat(code: string): boolean {
152152

153153
// Check if it contains '=>' or 'function('
154154
// If it does, it's a regular arrow function or function expression, not shorthand
155-
if (beforeBrace.includes('=>') || beforeBrace.includes('function(') || beforeBrace.includes('function (')) {
155+
// Use a simple check that handles common whitespace variations
156+
if (beforeBrace.includes('=>') || /function\s*\(/.test(beforeBrace)) {
156157
return false;
157158
}
158159

@@ -449,22 +450,22 @@ export class ModuleMinifierPlugin implements WebpackPluginInstance {
449450
// Remove from start up to and including '__DEFAULT_ID__'
450451
unwrapped.replace(0, shorthandPrefixEnd + defaultIdStr.length - 1, '');
451452
// Remove the suffix from the end
452-
// The suffix is '});' after minification (newline removed)
453-
// Look for '});' from the end
453+
// After minification, the suffix is typically '});' (3 characters, newline removed)
454454
const minifiedSuffix: string = '});';
455455
if (minified.endsWith(minifiedSuffix)) {
456456
unwrapped.replace(len - minifiedSuffix.length, len - 1, '');
457457
} else if (minified.endsWith(MODULE_WRAPPER_SHORTHAND_SUFFIX)) {
458458
// In case minifier keeps the newline
459459
unwrapped.replace(len - MODULE_WRAPPER_SHORTHAND_SUFFIX.length, len - 1, '');
460460
} else {
461-
// Fallback: try to find the closing
462-
unwrapped.replace(len - 3, len - 1, '');
461+
// Fallback: assume '});' suffix
462+
unwrapped.replace(len - minifiedSuffix.length, len - 1, '');
463463
}
464464
} else {
465465
// Fallback: If __DEFAULT_ID__ is not found (shouldn't happen), remove by length
466+
const minifiedSuffix: string = '});';
466467
unwrapped.replace(0, MODULE_WRAPPER_SHORTHAND_PREFIX.length - 1, '');
467-
unwrapped.replace(len - 3, len - 1, ''); // Remove '});'
468+
unwrapped.replace(len - minifiedSuffix.length, len - 1, '');
468469
}
469470
} else {
470471
// Regular format

0 commit comments

Comments
 (0)