Fixes making changes on JS imports #32626
Conversation
…at the check for alias symbol is re-applied - re comment in microsoft#26912
| if (target !== unknownSymbol) { | ||
| // For external modules symbol represent local symbol for an alias. | ||
|
|
||
| const shouldSkipWithJSRequireTargets = !isInJSFile(node) && moduleKind !== ModuleKind.ES2015; |
There was a problem hiding this comment.
I find an ad-hoc change like this suspect (despite our JS support being littered with them). You said the differences were caused by the symbol being bound with different flags in JS vs in TS - would that not imply that the appropriate fix lies in adjusting what flags the symbol is bound with, or using the symbol flags that are already different to change the behavior (and not walking up the node tree to find the context file)?
There was a problem hiding this comment.
OK makes sense, I've changed it - I'm a little worried now that before I felt like I definitely got what was going on previously, and now I partially get the change.
There was a problem hiding this comment.
The only flag you should need to check here is Assignment - Module comes from the thing being a module, and Alias (AliasExcludes === Alias, so, y'know, if you see AliasExcludes in the debug otuput it means Alias) from the thing being an import. Also, you should be able to check for that instead of isInJSFile(node) && moduleKind !== ModuleKind.ES2015, not in addition to.
There was a problem hiding this comment.
Ergh, I'm so sure I tried that earlier and got a tonne of fails. Thanks yeah, that's exactly what it needed.
There was a problem hiding this comment.
Yeah, the Assignment symbol flag is used to mean "allow unusual symbol merges, don't issue an error, deal with the result", since what you wanna suppress here is an error, checking just for that flag means sense.
orta
force-pushed
the
fix-31312
branch
3 times, most recently
from
87d87f7 to
66fac05
Compare
…nclude the expando'd objects
Fixes #31312 ( and #26912 )
The longer story can be found in #26912 - but essentially this short-circuits the check for alias symbols when making assignments to an import in JavaScript code
4145fef aims to address #26912 (comment)