Bind non-expando property assignments at top-level #26908
Conversation
Previously, only property assignments with expando initialisers were bound in top-level statements. Now, all property assignments are bound. This requires a matching change in the checker to make sure that these assignments remain context sensitive if their valueDeclaration is a 'real' declaration (ie a non assignment-declaration).
| @@ -1,12 +1,12 @@ | |||
| === tests/cases/conformance/salsa/a.js === | |||
| var variable = {}; | |||
| >variable : { a: number; } | |||
| >variable : typeof variable | |||
There was a problem hiding this comment.
Do we want to consider not using typeof on the printout of expando types if they don't contain call/construct signatures? Just a thought.
There was a problem hiding this comment.
Given that this is a JS scenario, I think that the "value name as type name" pattern is actually more understandable than a structural type -- for authors, though not for us. chrome-devtools-frontend, for example: would they rather see typeof Common or { Ns1: { ... }, Ns2: { ... } }?
There was a problem hiding this comment.
Hm, I think it depends on use. If it's being used like a namespace, then typeof is fine, but if it's being used like an expanding object literal, then the final type would probably be desirable. Seems kinda heuristic to me. Would be nice if we could display the detailed type in the editor on request, though.
| ==== tests/cases/conformance/salsa/bug24658.js (1 errors) ==== | ||
| import { hurk } from './mod1' | ||
| ~~~~ | ||
| !!! error TS2440: Import declaration conflicts with local declaration of 'hurk'. |
There was a problem hiding this comment.
Hm. Should we make this merge work in JS, rather than making it an error? We already allow import merges when the kinds don't conflict, and JS merges are like a merge-when-the-kinds-to-conflict.
There was a problem hiding this comment.
You can merge with an import? In the original discussion for this bug #24658, Mohamed and I just assumed that it was an error. Can you point me to the code/tests for import merges?
There was a problem hiding this comment.
Filed #26912 to track merging imports and assignment declarations
Previously, only property assignments with expando initialisers were bound in top-level statements. Now, all property assignments are bound.
This requires a matching change in the checker to make sure that these assignments remain context sensitive if their valueDeclaration is a 'real' declaration (ie a non assignment-declaration).
Note that this accords with the recent change of SymbolFlags.JSContainer → SymbolFlags.Assignment such that all assignment declarations are marked instead of just ones that could be a container later.
Fixes #26875