Skip to content

🤖 Cherry-pick PR #34513 into release-3.7 #34800

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

typescript-bot
Copy link
Collaborator

This cherry-pick was triggerd by a request on #34513
Please review the diff and merge if no changes are unexpected.
You can view the cherry-pick log here.

cc @andrewbranch

Component commits:
62aad54 Fix a crash when transforming functions in modules.
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.
@p-szm
Copy link
Contributor

p-szm commented Nov 9, 2019

@andrewbranch any plans to get this in?

@andrewbranch andrewbranch merged commit 93b1aa3 into microsoft:release-3.7 Nov 11, 2019
@andrewbranch
Copy link
Member

Thanks @patrickszmucer, almost forgot about this 😅

It will be in 3.7.3.

@p-szm
Copy link
Contributor

p-szm commented Nov 11, 2019

Thanks @andrewbranch! Do you know when 3.7.3 is going to be released?

@andrewbranch
Copy link
Member

The current plan is sometime this week. We have some more bugs to fix and throw in there first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants