diff --git a/change/@griffel-transform-04e458b3-6dac-4944-8d4e-f47b6c3501c3.json b/change/@griffel-transform-04e458b3-6dac-4944-8d4e-f47b6c3501c3.json new file mode 100644 index 000000000..6c6f27780 --- /dev/null +++ b/change/@griffel-transform-04e458b3-6dac-4944-8d4e-f47b6c3501c3.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "fix: wrap VM errors as host Error with filename context", + "packageName": "@griffel/transform", + "email": "olfedias@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/change/@griffel-transform-shaker-a871c8ee-56b7-4d17-b9f8-413cd0099166.json b/change/@griffel-transform-shaker-a871c8ee-56b7-4d17-b9f8-413cd0099166.json new file mode 100644 index 000000000..00444c28e --- /dev/null +++ b/change/@griffel-transform-shaker-a871c8ee-56b7-4d17-b9f8-413cd0099166.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "fix: guard control flow structural children (catch param, labels, loop tests) from removal", + "packageName": "@griffel/transform-shaker", + "email": "olfedias@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/packages/transform-shaker/src/__snapshots__/shaker.test.ts.snap b/packages/transform-shaker/src/__snapshots__/shaker.test.ts.snap index a39bb8df3..0353d86ba 100644 --- a/packages/transform-shaker/src/__snapshots__/shaker.test.ts.snap +++ b/packages/transform-shaker/src/__snapshots__/shaker.test.ts.snap @@ -12,6 +12,14 @@ exports[`keeps ESM import with batch export 1`] = ` export const __mkPreval = { color: colorBlue };" `; +exports[`keeps IIFE enum initializer when export is requested 1`] = ` +"export var ThemeName; +(function (ThemeName) { + ThemeName[ThemeName["Light"] = 0] = "Light"; + ThemeName[ThemeName["Dark"] = 1] = "Dark"; +})(ThemeName || (ThemeName = {}));" +`; + exports[`keeps all export-all re-exports when requested export is not found locally 1`] = ` "export * from './foo'; export * from './baz';" @@ -38,6 +46,13 @@ export const config = new Config();" exports[`keeps export const declarators 1`] = `"export const colorBlue = 'blue';"`; +exports[`keeps export default identifier referencing a function declaration 1`] = ` +"function isPlainObject(value) { + return typeof value === 'object'; +} +export default isPlainObject;" +`; + exports[`keeps export-all re-exports when referenced export is not found locally 1`] = ` "export * from './colors'; " @@ -143,6 +158,27 @@ const foo = bar(); export const __linariaPreval = [foo];" `; +exports[`preserves catch clause parameter when unused 1`] = ` +"function fn() { + try { throw new Error('test'); } + catch (e) { /* unused */ } +} +export const __linariaPreval = [fn];" +`; + +exports[`preserves labeled statement label 1`] = ` +"function fn() { + var result = []; + outer: + while (true) { + result.push(1); + break ; + } + return result; +} +export const __linariaPreval = [fn];" +`; + exports[`removes \`export * as ns\` if not requested 1`] = `"export * from './other';"`; exports[`removes all 1`] = ` @@ -194,6 +230,14 @@ const color2 = (local = color1(), () => local); export const __linariaPreval = [color2];" `; +exports[`shakes unrelated IIFE enums in the same file 1`] = ` +"export var ThemeName; +(function (ThemeName) { + ThemeName[ThemeName["Light"] = 0] = "Light"; + ThemeName[ThemeName["Dark"] = 1] = "Dark"; +})(ThemeName || (ThemeName = {}));" +`; + exports[`should keep member expression key 1`] = ` "const key = 'blue'; const obj = { blue: '#00F' }; diff --git a/packages/transform-shaker/src/langs/core.ts b/packages/transform-shaker/src/langs/core.ts index c8f0d57f4..928774f46 100644 --- a/packages/transform-shaker/src/langs/core.ts +++ b/packages/transform-shaker/src/langs/core.ts @@ -35,7 +35,6 @@ import type { ExportDefaultDeclaration, ExportAllDeclaration, CallExpression, - LogicalExpression, AssignmentExpression, } from 'oxc-parser'; @@ -151,27 +150,41 @@ function getAffectedNodes(node: Node, state: GraphBuilder): Node[] { return []; } +/** Unwraps `ParenthesizedExpression` wrappers (oxc-parser preserves these, Babel does not). */ +function unwrapParens(node: Node): Node { + while (node.type === 'ParenthesizedExpression' && (node as { expression: Node }).expression) { + node = (node as { expression: Node }).expression; + } + return node; +} + /* * In some cases (such as enums) babel uses CallExpression for object initializations * (function (Colors) { * Colors["BLUE"] = "#27509A"; * })(Colors || (Colors = {})); + * + * oxc-parser may wrap the RHS in a ParenthesizedExpression: `Colors || (Colors = {})`, + * so we unwrap before checking. */ -function isLazyInit(statement: ExpressionStatement): statement is ExpressionStatement & { - expression: CallExpression & { arguments: [LogicalExpression & { right: AssignmentExpression }] }; -} { +function isLazyInit(statement: ExpressionStatement): { assignmentNode: AssignmentExpression } | null { const { expression } = statement; if (!isCallExpression(expression) || expression.arguments.length !== 1) { - return false; + return null; } const [arg] = expression.arguments; if (!isLogicalExpression(arg) || arg.operator !== '||') { - return false; + return null; } const { left, right } = arg; - return isIdentifier(left) && isAssignmentExpression(right); + const unwrappedRight = unwrapParens(right); + if (isIdentifier(left) && isAssignmentExpression(unwrappedRight)) { + return { assignmentNode: unwrappedRight }; + } + + return null; } export const visitors = { @@ -181,8 +194,11 @@ export const visitors = { this.graph.addEdge(node, node.expression); this.graph.addEdge(node.expression, node); - if (isLazyInit(node)) { - this.graph.addEdge(node.expression.arguments[0].right, node); + const lazyInit = isLazyInit(node); + if (lazyInit) { + // The unwrapped AssignmentExpression (not the ParenthesizedExpression wrapper) must + // depend on the ExpressionStatement so that the IIFE stays alive when the variable is alive. + this.graph.addEdge(lazyInit.assignmentNode, node); } }, @@ -682,9 +698,11 @@ export const identifierHandlers: IdentifierHandlers = { ['ImportDefaultSpecifier', 'local'], ['ImportNamespaceSpecifier', 'local'], ['ExportSpecifier', 'local', 'exported'], - ['ExportDefaultDeclaration', 'declaration'], ], refer: [ + // `export default ` — the identifier references a variable declaration. + // (For `export default function/class`, declaration is not an Identifier so this handler won't fire.) + ['ExportDefaultDeclaration', 'declaration'], ['ArrayExpression', 'elements'], ['AssignmentExpression', 'left', 'right'], ['BinaryExpression', 'left', 'right'], diff --git a/packages/transform-shaker/src/shaker.test.ts b/packages/transform-shaker/src/shaker.test.ts index a1744f0c0..1e98e16c7 100644 --- a/packages/transform-shaker/src/shaker.test.ts +++ b/packages/transform-shaker/src/shaker.test.ts @@ -393,3 +393,72 @@ it('does not break export * as ns when requesting the namespace export', () => { expect(shaken).toMatchSnapshot(); }); + +it('preserves catch clause parameter when unused', () => { + const [shaken] = _shake()` + function fn() { + try { throw new Error('test'); } + catch (e) { /* unused */ } + } + export const __linariaPreval = [fn]; + `; + + expect(shaken).toMatchSnapshot(); +}); + +it('keeps export default identifier referencing a function declaration', () => { + const [shaken] = _shake(['default'])` + function isPlainObject(value) { + return typeof value === 'object'; + } + export default isPlainObject; + `; + + expect(shaken).toMatchSnapshot(); +}); + +it('keeps IIFE enum initializer when export is requested', () => { + const [shaken] = _shake(['ThemeName'])` + export var ThemeName; + (function (ThemeName) { + ThemeName[ThemeName["Light"] = 0] = "Light"; + ThemeName[ThemeName["Dark"] = 1] = "Dark"; + })(ThemeName || (ThemeName = {})); + `; + + expect(shaken).toMatchSnapshot(); +}); + +it('shakes unrelated IIFE enums in the same file', () => { + const [shaken] = _shake(['ThemeName'])` + export var ActionStyle; + (function (ActionStyle) { + ActionStyle["Default"] = "default"; + ActionStyle["Positive"] = "positive"; + })(ActionStyle || (ActionStyle = {})); + export var ThemeName; + (function (ThemeName) { + ThemeName[ThemeName["Light"] = 0] = "Light"; + ThemeName[ThemeName["Dark"] = 1] = "Dark"; + })(ThemeName || (ThemeName = {})); + `; + + expect(shaken).toMatchSnapshot(); +}); + +it('preserves labeled statement label', () => { + const [shaken] = _shake()` + function fn() { + var result = []; + outer: + while (true) { + result.push(1); + break outer; + } + return result; + } + export const __linariaPreval = [fn]; + `; + + expect(shaken).toMatchSnapshot(); +}); diff --git a/packages/transform-shaker/src/shaker.ts b/packages/transform-shaker/src/shaker.ts index 2c2aa4110..5afb17744 100644 --- a/packages/transform-shaker/src/shaker.ts +++ b/packages/transform-shaker/src/shaker.ts @@ -15,6 +15,18 @@ const STRUCTURAL_CHILDREN: Record> = { ExportNamedDeclaration: new Set(['source']), ExportAllDeclaration: new Set(['source', 'exported']), ImportDeclaration: new Set(['source']), + // Removing CatchClause.param turns `catch(e) {}` into `catch() {}` which is invalid syntax. + // Valid forms are `catch(e) {}` or `catch {}` (optional catch binding), but not `catch() {}`. + CatchClause: new Set(['param']), + // Removing LabeledStatement.label turns `outer: while(…)` into `: while(…)` which is invalid. + LabeledStatement: new Set(['label']), + // Removing loop/conditional test expressions produces invalid syntax (e.g. `while() {}`, `do {} while ()`). + DoWhileStatement: new Set(['test']), + WhileStatement: new Set(['test']), + IfStatement: new Set(['test']), + SwitchStatement: new Set(['discriminant']), + ForInStatement: new Set(['left', 'right']), + ForOfStatement: new Set(['left', 'right']), }; function isStatementBody(nodeType: string, key: string): boolean { diff --git a/packages/transform/src/utils/convertESMtoCJS.mts b/packages/transform/src/utils/convertESMtoCJS.mts index 61118b2d1..6e4b62b5d 100644 --- a/packages/transform/src/utils/convertESMtoCJS.mts +++ b/packages/transform/src/utils/convertESMtoCJS.mts @@ -63,6 +63,7 @@ export function convertESMtoCJS(code: string, filename: string): string { } const ms = new MagicString(code); + const deferredExports: string[] = []; for (const node of program.body) { switch (node.type) { @@ -129,13 +130,11 @@ export function convertESMtoCJS(code: string, filename: string): string { const names = (prop(decl, 'declarations') as Node[]).flatMap(d => extractDeclaredNames(prop(d, 'id') as Node), ); - const exportsCode = names.map(name => `exports.${name} = ${name};`).join(' '); - ms.appendLeft(node.end, '\n' + exportsCode); + deferredExports.push(...names); } else if (decl.type === 'FunctionDeclaration' || decl.type === 'ClassDeclaration') { const id = prop(decl, 'id') as Node | null; if (id) { - const name = prop(id, 'name') as string; - ms.appendLeft(node.end, `\nexports.${name} = ${name};`); + deferredExports.push(prop(id, 'name') as string); } } } else if (prop(node, 'source')) { @@ -213,6 +212,13 @@ export function convertESMtoCJS(code: string, filename: string): string { } } + // Append deferred exports at end-of-file so that IIFEs (e.g. TS compiled enums) + // can populate variables before they are captured by `exports.X = X`. + // ESM uses live bindings; deferring to end-of-file approximates that for CJS. + if (deferredExports.length > 0) { + ms.append('\n' + deferredExports.map(name => `exports.${name} = ${name};`).join('\n')); + } + // Mark the module as ESM-converted for interop ms.prepend('Object.defineProperty(exports, "__esModule", { value: true });\n'); diff --git a/packages/transform/src/utils/convertESMtoCJS.test.mts b/packages/transform/src/utils/convertESMtoCJS.test.mts index c3bcc3179..aaec6bb51 100644 --- a/packages/transform/src/utils/convertESMtoCJS.test.mts +++ b/packages/transform/src/utils/convertESMtoCJS.test.mts @@ -70,7 +70,8 @@ describe('convertESMtoCJS', () => { expect(convertESMtoCJS('export const { a, b } = obj;', '/test.js')).toMatchInlineSnapshot(` "Object.defineProperty(exports, "__esModule", { value: true }); const { a, b } = obj; - exports.a = a; exports.b = b;" + exports.a = a; + exports.b = b;" `); }); @@ -82,6 +83,24 @@ describe('convertESMtoCJS', () => { `); }); + it('defers export var assignment for TS enum IIFE pattern', () => { + const code = [ + 'export var Depths;', + '(function (Depths) {', + ' Depths.depth4 = "shadow";', + '})(Depths || (Depths = {}));', + ].join('\n'); + + expect(convertESMtoCJS(code, '/test.js')).toMatchInlineSnapshot(` + "Object.defineProperty(exports, "__esModule", { value: true }); + var Depths; + (function (Depths) { + Depths.depth4 = "shadow"; + })(Depths || (Depths = {})); + exports.Depths = Depths;" + `); + }); + it('converts export { name }', () => { expect(convertESMtoCJS('const x = 1;\nexport { x };', '/test.js')).toMatchInlineSnapshot(` "Object.defineProperty(exports, "__esModule", { value: true });