In JS, fix property access on imports of aliased exports#24930
In JS, fix property access on imports of aliased exports#24930sandersn wants to merge 4 commits into
Conversation
| if (symbol.flags & SymbolFlags.Transient && hasEntries(symbol.exports) && widened.flags & TypeFlags.StructuredType) { | ||
| resolveStructuredTypeMembers(widened as StructuredType); | ||
| const exports = createSymbolTable(); | ||
| forEachDeclarationInChain(decl, s => { |
There was a problem hiding this comment.
Does this need to be done up-front, or can it be deferred until members are requested in resolveStructuredTypeMembers on the result of this call? The resolveStructuredTypeMembers on the other intersection member (to look for is members to check for things to merge) above leads me to believe that it may be good to do so. AFAIK, we'd need to set a CheckFlag or something on this intersection's symbol (or maybe its anonymous object symbol?) and then move this work into resolveStructuredTypeMembers.
There was a problem hiding this comment.
Fair enough. I moved resolution to resolveIntersectionTypeMembers, which is still slightly less lazy than it could be -- currently resolveIntersectionTypeMembers doesn't resolve members, just call and index signatures -- but seems like a good improvement.
Except that it now incorrectly applies to a couple of global merged symbols that have nothing to do with module.exports, so I need to fix those. I'll push a commit when I've fixed that problem.
|
@weswigham All right, want to take a look now that it's delayed until intersection-property-resolution time? |
| } | ||
|
|
||
| function resolveSyntheticJSModuleExportMembers(type: IntersectionType) { | ||
| if (type.types.length === 2) { |
There was a problem hiding this comment.
This is a bad predicate. Intersection types are flattened, so if I have one of these intersections and intersect with a 3rd thing, the members from the js initializer will no longer resolve. (eg, /* @type {typeof jsContainer & MyThing} */)
You probably wanna do the filtering/combining on CheckFlags.SyntheticExportEqual in the main resolveIntersectionTypeMembers block.
There was a problem hiding this comment.
Woooooops. Nice catch, I forgot all about that. Upon reflection, I decided that removing duplicates from the intersection members wasn't worth that much work anyway. I think it's OK to have duplicated properties of an intersection in a [probably] rare case like this.
I removed resolveSyntheticJSModuleExportMembers and updated the baselines.
|
@weswigham Mind taking another look? |
|
This now works; I think I fixed it when I improved the typing of |
Complete the fix of #24754, fixing the incorrect error when accessing properties on the imported symbol. #24871 fixed the incorrect error when accessing properties on
exportsin the declaring file:Much like merging IIFE JS containers, I have to use intersection to implement the merge of
export=and exported properties, because the type ofexport=could be anything returnable by an IIFE.