fix(check/lsp): fix bugs with tsc type resolution, allow npm packages to augment ImportMeta
#27690
Conversation
|
Great work, this is very exciting!! |
|
Two tests are failing because TSC is emitting a new error: The tests are essentially What happens is that we resolve the type reference directive to I think we can safely ignore this diagnostic because we emit our own not found errors, but not 100% sure. |
| let specifier_str = original.replace(".d.ts.d.ts", ".d.ts"); | ||
| let specifier_str = original | ||
| .replace(".d.ts.d.ts", ".d.ts") | ||
| .replace("$node_modules", "node_modules"); |
There was a problem hiding this comment.
Huh, I kept trying this as a hail mary for some other bugs but could never manifest a case where tsc was constructing paths bypassing op_resolve() (which typically stores mappings for every specifier when denormalizing its results). I guess it's happening at host.resolveTypeReferenceDirectiveReferences().
Nice find!
| typeof diagnostic.messageText === "string" && | ||
| (importMetaFilenameDirnameTypesRe.test(diagnostic.messageText)) && | ||
| (diagnostic.file?.fileName.startsWith("asset:///") || | ||
| diagnostic.file?.fileName?.includes("@types/node")) |
There was a problem hiding this comment.
Does this work for the global cache?
There was a problem hiding this comment.
Added a test for this (across all of the node modules modes)
There was a problem hiding this comment.
Ah, I misremembered how the global cache was structured.
… to augment `ImportMeta` (#27690) Fixes #26224. Fixes #27042. There were three bugs here: - we were only resolving `/// <reference types` directives starting with `npm:`, which meant we failed to resolve bare specifiers (this broke the `/// <reference types="vite/client">` directive in most of the vite templates) - the `$node_modules` workaround caused us to fail to read files for tsc. For instance tsc would construct new paths based on specifiers containing `$node_modules`, and since we hadn't created those we weren't mapping them back to the original (this broke some type resolution within `vite/client`) - our separation of `ImportMeta` across node and deno globals in tsc meant that npm packages couldn't augment `ImportMeta` (this broke `vite/client`'s augmentation to add `import.meta.env` and others) After this, the only remaining issue in the vanilla vite template is our error on `/vite.svg` (which is an ambient module), and I'll look into that next.
Fixes #26224.
Fixes #27042.
There were three bugs here:
/// <reference typesdirectives starting withnpm:, which meant we failed to resolve bare specifiers (this broke the/// <reference types="vite/client">directive in most of the vite templates)$node_modulesworkaround caused us to fail to read files for tsc. For instance tsc would construct new paths based on specifiers containing$node_modules, and since we hadn't created those we weren't mapping them back to the original (this broke some type resolution withinvite/client)ImportMetaacross node and deno globals in tsc meant that npm packages couldn't augmentImportMeta(this brokevite/client's augmentation to addimport.meta.envand others)After this, the only remaining issue in the vanilla vite template is our error on
/vite.svg(which is an ambient module), and I'll look into that next.