This repository has been archived by the owner on Feb 26, 2024. It is now read-only.
Fix exports of local aliases of the form exports.B = exports.A.
#912
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
evmar
approved these changes
Aug 23, 2019
| String emitName = Constants.INTERNAL_NAMESPACE + "."; | ||
| // If the alias is to a local name, but prefixed with exports.Foo, | ||
| // we need to rewrite "exports" because it is no longer correct in TS. | ||
| if (alternativeAliasName.startsWith("exports")) { |
There was a problem hiding this comment.
exports.
with the trailing dot, so you don't match exportsTheWorld etc.
|
On a second thought, a better fix would be to simply not add |
f09cbef
to
c6c511b
Compare
|
Rewrote this to a much simpler fix. Instead of emitting a self-import, just skip the syntactic alias rewriting step. Turns out the JS compiler output is adequate we were just clobber it previously. PTAL. |
evmar
approved these changes
Aug 26, 2019
It appears that the emit was previously very broken for this case (syntactically incorrect). This change simply skips the alias rewriting when "... = exports...." pattern is detected syntactically. For the rest we still assume the syntactic rewriter is desirable, like the case `exports.A = externalModule.Foo`.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It appears that the emit was previously very broken for this case
(syntactically incorrect). This change fixes the emit by detecting
rewrites that mention
exports.Fooand rewrite them to thecorresponding clutz emitted namespace.
Note, that the test .d.ts is still partially incorrect, because we
create a type/let pair for just for a let binding. With #896 this will
be fully correct.