Skip to content

Commit ff590b6

Browse files
mprobstandrewbranch
authored andcommitted
Fix a crash when transforming functions in modules. (microsoft#34513)
When transforming a module declaration and block, parse tree nodes contained in the module block have their parent pointers reset due to `shouldEmitModuleDeclaration` calling into `isInstantiatedModule`, which needs to set parent pointers to operate. That causes a crash when later transforming any nodes within the module, as retrieving their source file in `getSourceFileOfNode` (via `getOrCreateEmitNode`) fails, due to their new synthesized parent nodes not being in a source file. This change avoids the issue by using the parse tree node in `ts.ts` to decide whether a module declaration should be emitted (i.e. whether the module contains values). This means transformers cannot add values to modules that previously did not contain any. Fixes microsoft#34644.
1 parent cbbbcfa commit ff590b6

File tree

3 files changed

+40
-1
lines changed

3 files changed

+40
-1
lines changed

src/compiler/transformers/ts.ts

+6-1
Original file line numberDiff line numberDiff line change
@@ -2452,7 +2452,12 @@ namespace ts {
24522452
*
24532453
* @param node The module declaration node.
24542454
*/
2455-
function shouldEmitModuleDeclaration(node: ModuleDeclaration) {
2455+
function shouldEmitModuleDeclaration(nodeIn: ModuleDeclaration) {
2456+
const node = getParseTreeNode(nodeIn, isModuleDeclaration);
2457+
if (!node) {
2458+
// If we can't find a parse tree node, assume the node is instantiated.
2459+
return true;
2460+
}
24562461
return isInstantiatedModule(node, !!compilerOptions.preserveConstEnums || !!compilerOptions.isolatedModules);
24572462
}
24582463

src/testRunner/unittests/transform.ts

+29
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,35 @@ namespace Foo {
447447
}).outputText;
448448
});
449449

450+
testBaseline("transformUpdateModuleMember", () => {
451+
return transpileModule(`
452+
module MyModule {
453+
const myVariable = 1;
454+
function foo(param: string) {}
455+
}
456+
`, {
457+
transformers: {
458+
before: [renameVariable],
459+
},
460+
compilerOptions: {
461+
target: ScriptTarget.ES2015,
462+
newLine: NewLineKind.CarriageReturnLineFeed,
463+
}
464+
}).outputText;
465+
466+
function renameVariable(context: TransformationContext) {
467+
return (sourceFile: SourceFile): SourceFile => {
468+
return visitNode(sourceFile, rootTransform, isSourceFile);
469+
};
470+
function rootTransform<T extends Node>(node: T): Node {
471+
if (isVariableDeclaration(node)) {
472+
return updateVariableDeclaration(node, createIdentifier("newName"), /* type */ undefined, node.initializer);
473+
}
474+
return visitEachChild(node, rootTransform, context);
475+
}
476+
}
477+
});
478+
450479
// https://github.com/Microsoft/TypeScript/issues/24709
451480
testBaseline("issue24709", () => {
452481
const fs = vfs.createFromFileSystem(Harness.IO, /*caseSensitive*/ true);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
var MyModule;
2+
(function (MyModule) {
3+
const newName = 1;
4+
function foo(param) { }
5+
})(MyModule || (MyModule = {}));

0 commit comments

Comments
 (0)