Fix CJS export of typedef and class w/latebound names#55053
Fix CJS export of typedef and class w/latebound names#55053sandersn merged 3 commits intomicrosoft:mainfrom
Conversation
Fixes microsoft#53967 The problem is that the weird, ad-hoc CJS merge needs to happen at two different times. The first is its existing location in getCommonJsExportEquals. The second is getResolvedMembersOrExportsOfSymbol, which combines normal, early-bound symbols with late-bound symbols. The CJS merge happens before the addition of late-bound symbols, so a merge will result in the loss of [Symbol.iterable] from the bug. But manually merging the resolvedExports/Members during the CJS merge also doesn't work, because it prevents the module's resolvedExports/Members from being filled in later. Instead, this PR delays part of the CJS merge to getResolvedMembersOrExportsOfSymbol. Unfortunately, this requires retrieving the original, unmerged symbols--since their resolvedExports/Members are already cached. I do this by looking at the symbols for each of the merged symbol's declarations, which feels wrong. Please suggest alternate ways to do this. I don't think I understand the late-binding machinery well enough, so it's likely there's a cleaner way.
weswigham
left a comment
There was a problem hiding this comment.
I don't think I understand the late-binding machinery well enough, so it's likely there's a cleaner way.
The cleaner way is probably breaking the cycle between late-binding and cjs export merging, I think by making it so module members bind directly into the export= symbol if it's present (which probably requires a 2-pass process), rather than the containing module - basically moving what getCommonJsExportEquals does up into the binder phase. There's probably a bunch of knock-on effects to this, though, since I know the js declaration emitter, at least, already does a bunch of hacks (or at least stuff similar to this where it goes back and looks at the original unmerged symbols) to work around the odd symbol structure getCommonJsExportEquals creates.
In any case, this is probably fine - it's just piling a similar workaround into an already ugly space in the compiler - but maybe @rbuckton has more thoughts from the late bound member angle?
Fixes #53967
The problem is that the weird, ad-hoc CJS merge needs to happen at two different times. The first is its existing location in getCommonJsExportEquals. The second is getResolvedMembersOrExportsOfSymbol, which combines normal, early-bound symbols with late-bound symbols.
The CJS merge happens before the addition of late-bound symbols, so a merge will result in the loss of [Symbol.iterable] from the bug. But manually merging the resolvedExports/Members during the CJS merge also doesn't work, because it prevents the module's resolvedExports/Members from being filled in later.
Instead, this PR delays part of the CJS merge to
getResolvedMembersOrExportsOfSymbol. Unfortunately, this requires retrieving the original, unmerged symbols--since their resolvedExports/Members are already cached. I do this by looking at the symbols for each of the merged symbol's declarations, which feels wrong.
Please suggest alternate ways to do this. I don't think I understand the late-binding machinery well enough, so it's likely there's a cleaner way.