Hold back squiggling code fragments until all symbols have been imported

[moniuch]

This code below is all squiggled as if the entire fragment was completely wrong, eg. messed up by an unbalanced parenthesis, or a missing comma.

Meanwhile, this is just because of a missing import of map from rxjs. The TS typing error messages weren't helpful enough to make me immediately spot the problem.

What I am asking here is to change the principle behind rendering of the squiggles to the following: don't mark any fragments of code before all symbols have been known. First flag missing imports only, once this is done, go and mark the squiggles as need be.

(Could be that there is a theme-related setting that I am unaware of, which can render double squiggles: one run for the unknown symbol, another for the code error. If so, I could adjust my theme with different styles for each runs. If so, then please share the name of the theme setting with me.)

Not sure why the cmd-. auto-fix item for bringing all missing imports is not available, either.

[mjbvz]

Can you please share self-contained minimal example code that demonstrates the issue

[moniuch]

@mjbvz Hi, I managed to reproduce it on Stackblitz. Hope that helps

https://stackblitz.com/edit/angular-ivy-yb4xua?file=src%2Fapp%2Fstore%2Feffects.ts

Just comment out the indicated line in effects.ts

A word of explanation in case you are not familiar with angular/ngrx: createEffect by default takes a function argument whose return type should be an Action, which is what the map does in this example map(() => fromAppActions.IncrementComplete()) So, when imports are commented (say, map), the whole type flow breaks and gets squiggled top to bottom. Meanwhile, the code itself is ok, it's just missing the necessary imports. Unfortunately, it is hard to tell which imports are missing, because the squiggles are not indicating them. So you need to go one by one and check.

Bottomline, if some imports are missing, put the squiggles just on those - without these imports in place it is impossible to tell that the whole code is wrong, so the whole code should not be marked in its entirety.

[mjbvz]

Thanks @moniuch

Here's a simple repo of the problem:

export function foo(x: string) { }

foo({ x: noSuchSymbol })

Someone on the TS team can weigh in too but I'll add my thoughts

We already surface two errors here: one for the missing symbol missing and one for the incorrect type. My ideas on improving this:

• Figure out a better UX for showing overlapping error spans on the VS Code side (previously discussed on https://github.com/microsoft/vscode/issues/88514)

• Keep both errors but make the outer one clearer that it involves an unresolved symbol and fixing that may fix the problem (potentially using related error spans)

---

As a side note, I do see the add missing import quick fix when my cursor is the missing symbol name (map):

However you can't trigger this fix on the outer error

[moniuch]

@mjbvz I am by no way an expert in TS, but IMO:

one for the missing symbol missing and one for the incorrect type

It is not possible to evaluate the type as correct or incorrect before you have all imports in place. Hence my ticket title: first flag missing imports only, flag only these code "units" where you don't have missing imports. IMO, very logical and clutter-free.

Your side note also points to another productivity issue - the caret position required for the import fix is unnecessarily strict. I believe, under the hood it is clear for the language service that there are some imports still unresolved, in which situation the Add all missing imports fix should be available regardless of the caret position. Please correct me if I am wrong in my assumption. Relaxed position wherever possible would save developers from unnecessary navigation/mousing around the code.

[fatcerberus]

@moniuch In the specific example given:

export function foo(x: string) { }

foo({ x: noSuchSymbol })

The typing here is known to be incorrect regardless of the type of noSuchSymbol, since the function wants a string. Thus the type error here is still useful (arguably more useful than the unresolved symbol error, even, since there's no sense fixing the latter, e.g., by adding imports, if I'm still going to have a type error afterwards). If the call is changed instead to foo(noSuchSymbol) then the type error goes away (the unresolved symbol is treated as any).

[moniuch]

@fatcerberus As I did not author the simplified snippet that you are quoting, I am not claiming it is correct or conclusive. My belief is that the code from the OP should not be squiggled while some of the imports within it were still missing.