Agoric / agoric-sdk

monorepo for the Agoric Javascript smart contract platform
Apache License 2.0
327 stars 207 forks source link

Lots of red squiggles in fallout from move away from ambient types #4444

Closed erights closed 2 years ago

erights commented 2 years ago

red-squiggles

On a clean master, even after restarting vscode and restarting the ts server and verifying the vscode is using ts version 4.5.5 and that this is also the workspace version, I see lot of red squiggles, which seems to be due to types whose def has moved to endojs. As we see here, IIUC, store/src/types.js still assumes that Passable is ambient. Is this no longer working? Why does vscode complain about this but not yarn lint?

mhofman commented 2 years ago

FYI, we didn't move from ambient types. This is a fallout from moving the marshal package to endo, and the fix is actually to move away from ambient types and to generated declarations, which is what #4413 does.

erights commented 2 years ago

Back in the days when we were only using ambient types, we had uses in agoric-sdk of types defined in endojs without these red squiggles. So I am confused about what this means.

This is not to dispute that we should move to non-ambient types. I still agree that we should. But depending on what I'm confused about, perhaps we can get back to a working no-squiggle state incrementally? Or is there a repair PR already ready to go with a full non-incremental fix?

mhofman commented 2 years ago

So I am confused about what this means.

It's possible TS server became less aggressive about how far down node_modules to parse .js files.

Or is there a repair PR already ready to go with a full non-incremental fix?

That PR is pretty much ready. We need another round of reviews, rebasing, and decide when to publish.

mhofman commented 2 years ago

Also a quick fix is to add import '@endo/marshal/exported.js'; to packages/store/src/types.js

erights commented 2 years ago

... quick fix ... Great!

Is there an easy way to find all the places this needs to be added to make red squiggles go away, without visually inspecting all files in vscode for red squiggles?

mhofman commented 2 years ago

I honestly have no idea

erights commented 2 years ago

Once #4447 is merged, should I leave this open awaiting a proper fix?

mhofman commented 2 years ago

Nope. I set the PR to close fix this issue