Skip to content

Commit 2a20639

Browse files
Copilotdmichon-msft
andcommitted
Fix syntax errors by re-enabling shorthand detection and skipping token asset minification
- Re-enabled isMethodShorthandFormat() detection (webpack emits modules without function keyword) - Modules wrapped as __MINIFY_MODULE__({__DEFAULT_ID__(__params__){body}}); for shorthand format - Assets with tokens skip minification to avoid syntax errors - Added WebpackOutputFormats tests to verify code sent to minifier - All tests passing with ZERO syntax errors Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>
1 parent e536fb7 commit 2a20639

5 files changed

Lines changed: 930 additions & 134 deletions

File tree

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

Lines changed: 106 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -140,10 +140,26 @@ function isLicenseComment(comment: Comment): boolean {
140140
* @returns true if the code is in method shorthand format
141141
*/
142142
function isMethodShorthandFormat(code: string): boolean {
143-
// Method shorthand detection is not currently needed as webpack doesn't emit true method shorthand yet
144-
// Webpack emits either function expressions or arrow functions, both of which should use regular wrapping
145-
// This function is kept for future compatibility when webpack adds method shorthand support
146-
return false;
143+
// Find the position of the first opening brace
144+
const firstBraceIndex: number = code.indexOf('{');
145+
if (firstBraceIndex === -1) {
146+
// No brace found, not a function format
147+
return false;
148+
}
149+
150+
// Get the code before the first brace
151+
const beforeBrace: string = code.slice(0, firstBraceIndex);
152+
153+
// Check if it contains '=>' or 'function('
154+
// If it does, it's a regular arrow function or function expression, not shorthand
155+
// Use a simple check that handles common whitespace variations
156+
if (beforeBrace.includes('=>') || /function\s*\(/.test(beforeBrace)) {
157+
return false;
158+
}
159+
160+
// If neither '=>' nor 'function(' are found, assume method shorthand format
161+
// This handles the case where webpack emits: (__params__) { body } without function keyword
162+
return true;
147163
}
148164

149165
/**
@@ -499,68 +515,100 @@ export class ModuleMinifierPlugin implements WebpackPluginInstance {
499515

500516
// Verify that this is a JS asset
501517
if (isJSAsset.test(assetName)) {
502-
++pendingMinificationRequests;
503-
504-
const { source: wrappedCodeRaw, map } = useSourceMaps
505-
? asset.sourceAndMap()
506-
: {
507-
source: asset.source(),
508-
map: undefined
509-
};
510-
511-
const rawCode: string = wrappedCodeRaw.toString();
512-
const nameForMap: string = `(chunks)/${assetName}`;
513-
514-
const hash: string = hashCodeFragment(rawCode);
515-
516-
minifier.minify(
517-
{
518-
hash,
519-
code: rawCode,
520-
nameForMap: useSourceMaps ? nameForMap : undefined,
521-
externals: undefined
522-
},
523-
(result: IModuleMinificationResult) => {
524-
if (isMinificationResultError(result)) {
525-
compilation.errors.push(result.error as WebpackError);
526-
// eslint-disable-next-line no-console
527-
console.error(result.error);
528-
} else {
529-
try {
530-
const { code: minified, map: minifierMap } = result;
531-
532-
const rawOutput: sources.Source = useSourceMaps
533-
? new SourceMapSource(
534-
minified, // Code
535-
nameForMap, // File
536-
minifierMap ?? undefined, // Base source map
537-
rawCode, // Source from before transform
538-
map ?? undefined, // Source Map from before transform
539-
true // Remove original source
540-
)
541-
: new RawSource(minified);
542-
543-
const withIds: sources.Source = postProcessCode(new ReplaceSource(rawOutput), {
544-
compilation,
545-
module: undefined,
546-
loggingName: assetName
547-
});
548-
518+
// Check if asset contains module tokens (which would make it invalid for minification)
519+
const assetSource: string = asset.source().toString();
520+
const hasTokens: boolean = assetSource.includes(CHUNK_MODULE_TOKEN);
521+
522+
if (hasTokens) {
523+
// Asset contains tokens - don't try to minify it, just store for rehydration
524+
minifiedAssets.set(assetName, {
525+
source: postProcessCode(new ReplaceSource(asset), {
526+
compilation,
527+
module: undefined,
528+
loggingName: assetName
529+
}),
530+
chunk,
531+
fileName: assetName,
532+
renderInfo: new Map(),
533+
type: 'javascript'
534+
});
535+
} else {
536+
// Asset doesn't have tokens - safe to minify
537+
++pendingMinificationRequests;
538+
539+
const { source: wrappedCodeRaw, map } = useSourceMaps
540+
? asset.sourceAndMap()
541+
: {
542+
source: asset.source(),
543+
map: undefined
544+
};
545+
546+
const rawCode: string = wrappedCodeRaw.toString();
547+
const nameForMap: string = `(chunks)/${assetName}`;
548+
549+
const hash: string = hashCodeFragment(rawCode);
550+
551+
minifier.minify(
552+
{
553+
hash,
554+
code: rawCode,
555+
nameForMap: useSourceMaps ? nameForMap : undefined,
556+
externals: undefined
557+
},
558+
(result: IModuleMinificationResult) => {
559+
if (isMinificationResultError(result)) {
560+
compilation.errors.push(result.error as WebpackError);
561+
// eslint-disable-next-line no-console
562+
console.error(result.error);
563+
// Store unminified asset as fallback
549564
minifiedAssets.set(assetName, {
550-
source: new CachedSource(withIds),
565+
source: postProcessCode(new ReplaceSource(asset), {
566+
compilation,
567+
module: undefined,
568+
loggingName: assetName
569+
}),
551570
chunk,
552571
fileName: assetName,
553572
renderInfo: new Map(),
554573
type: 'javascript'
555574
});
556-
} catch (err) {
557-
compilation.errors.push(err);
575+
} else {
576+
try {
577+
const { code: minified, map: minifierMap } = result;
578+
579+
const rawOutput: sources.Source = useSourceMaps
580+
? new SourceMapSource(
581+
minified, // Code
582+
nameForMap, // File
583+
minifierMap ?? undefined, // Base source map
584+
rawCode, // Source from before transform
585+
map ?? undefined, // Source Map from before transform
586+
true // Remove original source
587+
)
588+
: new RawSource(minified);
589+
590+
const withIds: sources.Source = postProcessCode(new ReplaceSource(rawOutput), {
591+
compilation,
592+
module: undefined,
593+
loggingName: assetName
594+
});
595+
596+
minifiedAssets.set(assetName, {
597+
source: new CachedSource(withIds),
598+
chunk,
599+
fileName: assetName,
600+
renderInfo: new Map(),
601+
type: 'javascript'
602+
});
603+
} catch (err) {
604+
compilation.errors.push(err);
605+
}
558606
}
559-
}
560607

561-
onFileMinified();
562-
}
563-
);
608+
onFileMinified();
609+
}
610+
);
611+
}
564612
} else {
565613
// This isn't a JS asset. Don't try to minify the asset wrapper, though if it contains modules, those might still get replaced with minified versions.
566614
minifiedAssets.set(assetName, {
Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
2+
// See LICENSE in the project root for license information.
3+
4+
jest.disableAutomock();
5+
import { promisify } from 'node:util';
6+
7+
import webpack, { type Stats, type InputFileSystem, type OutputFileSystem } from 'webpack';
8+
import { Volume } from 'memfs/lib/volume';
9+
10+
import { ModuleMinifierPlugin } from '../ModuleMinifierPlugin';
11+
import { MockMinifier } from './MockMinifier';
12+
13+
jest.setTimeout(1e9);
14+
15+
describe('WebpackOutputFormats', () => {
16+
it('Captures code sent to minifier with arrowFunction=false', async () => {
17+
const mockMinifier: MockMinifier = new MockMinifier();
18+
const memoryFileSystem: Volume = new Volume();
19+
memoryFileSystem.fromJSON(
20+
{
21+
'/package.json': '{}',
22+
'/entry.js': `import('./module.js').then(m => m.test());`,
23+
'/module.js': `export function test() { console.log("test"); }`
24+
},
25+
'/src'
26+
);
27+
28+
const minifierPlugin: ModuleMinifierPlugin = new ModuleMinifierPlugin({
29+
minifier: mockMinifier
30+
});
31+
32+
const compiler: webpack.Compiler = webpack({
33+
entry: '/entry.js',
34+
output: {
35+
path: '/release',
36+
filename: 'bundle.js',
37+
environment: {
38+
arrowFunction: false,
39+
const: false,
40+
destructuring: false,
41+
forOf: false,
42+
module: false
43+
}
44+
},
45+
optimization: {
46+
minimizer: []
47+
},
48+
context: '/',
49+
mode: 'production',
50+
plugins: [minifierPlugin]
51+
});
52+
53+
compiler.inputFileSystem = memoryFileSystem as unknown as InputFileSystem;
54+
compiler.outputFileSystem = memoryFileSystem as unknown as OutputFileSystem;
55+
56+
const stats: Stats | undefined = await promisify(compiler.run.bind(compiler))();
57+
await promisify(compiler.close.bind(compiler));
58+
if (!stats) {
59+
throw new Error(`Expected stats`);
60+
}
61+
const { errors, warnings } = stats.toJson('errors-warnings');
62+
expect(errors).toMatchSnapshot('Errors');
63+
expect(warnings).toMatchSnapshot('Warnings');
64+
65+
// Capture what was sent to the minifier
66+
const requests: Array<[string, string]> = Array.from(mockMinifier.requests.entries());
67+
// Just check that modules have the expected wrapper
68+
// eslint-disable-next-line @typescript-eslint/no-unused-vars
69+
for (const [_hash, code] of requests) {
70+
if (code.includes('__MINIFY_MODULE__')) {
71+
expect(code).toMatch(/^__MINIFY_MODULE__\(/);
72+
expect(code).toMatch(/\);$/);
73+
// Check if it's function or arrow
74+
if (code.includes('function')) {
75+
expect(code).toContain('function');
76+
} else if (code.includes('=>')) {
77+
expect(code).toContain('=>');
78+
}
79+
}
80+
}
81+
expect(requests).toMatchSnapshot('Minifier Requests');
82+
});
83+
84+
it('Captures code sent to minifier with arrowFunction=true', async () => {
85+
const mockMinifier: MockMinifier = new MockMinifier();
86+
const memoryFileSystem: Volume = new Volume();
87+
memoryFileSystem.fromJSON(
88+
{
89+
'/package.json': '{}',
90+
'/entry.js': `import('./module.js').then(m => m.test());`,
91+
'/module.js': `export function test() { console.log("test"); }`
92+
},
93+
'/src'
94+
);
95+
96+
const minifierPlugin: ModuleMinifierPlugin = new ModuleMinifierPlugin({
97+
minifier: mockMinifier
98+
});
99+
100+
const compiler: webpack.Compiler = webpack({
101+
entry: '/entry.js',
102+
output: {
103+
path: '/release',
104+
filename: 'bundle.js',
105+
environment: {
106+
arrowFunction: true,
107+
const: true,
108+
destructuring: true,
109+
forOf: true,
110+
module: false
111+
}
112+
},
113+
optimization: {
114+
minimizer: []
115+
},
116+
context: '/',
117+
mode: 'production',
118+
plugins: [minifierPlugin]
119+
});
120+
121+
compiler.inputFileSystem = memoryFileSystem as unknown as InputFileSystem;
122+
compiler.outputFileSystem = memoryFileSystem as unknown as OutputFileSystem;
123+
124+
const stats: Stats | undefined = await promisify(compiler.run.bind(compiler))();
125+
await promisify(compiler.close.bind(compiler));
126+
if (!stats) {
127+
throw new Error(`Expected stats`);
128+
}
129+
const { errors, warnings } = stats.toJson('errors-warnings');
130+
expect(errors).toMatchSnapshot('Errors');
131+
expect(warnings).toMatchSnapshot('Warnings');
132+
133+
// Capture what was sent to the minifier
134+
const requests: Array<[string, string]> = Array.from(mockMinifier.requests.entries());
135+
// Just check that modules have the expected wrapper
136+
// eslint-disable-next-line @typescript-eslint/no-unused-vars
137+
for (const [_hash, code] of requests) {
138+
if (code.includes('__MINIFY_MODULE__')) {
139+
expect(code).toMatch(/^__MINIFY_MODULE__\(/);
140+
expect(code).toMatch(/\);$/);
141+
// Check if it's function or arrow
142+
if (code.includes('function')) {
143+
expect(code).toContain('function');
144+
} else if (code.includes('=>')) {
145+
expect(code).toContain('=>');
146+
}
147+
}
148+
}
149+
expect(requests).toMatchSnapshot('Minifier Requests');
150+
});
151+
});

0 commit comments

Comments
 (0)