Skip to content

Commit

Permalink
Fix a crash when transforming functions in modules.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mprobst committed Oct 29, 2019
1 parent cbbbcfa commit 62aad54
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 1 deletion.
7 changes: 6 additions & 1 deletion src/compiler/transformers/ts.ts
Expand Up @@ -2452,7 +2452,12 @@ namespace ts {
*
* @param node The module declaration node.
*/
function shouldEmitModuleDeclaration(node: ModuleDeclaration) {
function shouldEmitModuleDeclaration(nodeIn: ModuleDeclaration) {
const node = getParseTreeNode(nodeIn, isModuleDeclaration);
if (!node) {
// If we can't find a parse tree node, assume the node is instantiated.
return true;
}
return isInstantiatedModule(node, !!compilerOptions.preserveConstEnums || !!compilerOptions.isolatedModules);
}

Expand Down
29 changes: 29 additions & 0 deletions src/testRunner/unittests/transform.ts
Expand Up @@ -447,6 +447,35 @@ namespace Foo {
}).outputText;
});

testBaseline("transformUpdateModuleMember", () => {
return transpileModule(`
module MyModule {
const myVariable = 1;
function foo(param: string) {}
}
`, {
transformers: {
before: [renameVariable],
},
compilerOptions: {
target: ScriptTarget.ES2015,
newLine: NewLineKind.CarriageReturnLineFeed,
}
}).outputText;

function renameVariable(context: TransformationContext) {
return (sourceFile: SourceFile): SourceFile => {
return visitNode(sourceFile, rootTransform, isSourceFile);
};
function rootTransform<T extends Node>(node: T): Node {
if (isVariableDeclaration(node)) {
return updateVariableDeclaration(node, createIdentifier("newName"), /* type */ undefined, node.initializer);
}
return visitEachChild(node, rootTransform, context);
}
}
});

// https://github.com/Microsoft/TypeScript/issues/24709
testBaseline("issue24709", () => {
const fs = vfs.createFromFileSystem(Harness.IO, /*caseSensitive*/ true);
Expand Down
@@ -0,0 +1,5 @@
var MyModule;
(function (MyModule) {
const newName = 1;
function foo(param) { }
})(MyModule || (MyModule = {}));

0 comments on commit 62aad54

Please sign in to comment.