Repository navigation
feat: typescript 7 content mapper - #3111
jasonlyu123 wants to merge 55 commits into
Conversation
…nto ts7-content-mapper
🦋 Changeset detectedLatest commit: d3766bc The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
no document highlight yet, VSCode doesn't seems to be merge results from ts and ours
|
Exciting progress! Do you have a stack trace for the auto-import panic handy? |
Weirdly, in import { SvelteComponentTyped } from te-tsgo-t;And if I track down the module specifier printing in that function, in the Btw, should I open an issue to track this? There is also a problem with duplicated inlay hints that seems to happen when a node is split into multiple spans. Should I open two separate issues? |
|
Two issues would be great, if you don't mind! |
we did this in the language-server too
|
Hey @jasonlyu123! Would love to get your input on two things:
We haven't hammered out the details on these, and I think you seem to be one of the farthest along in implementing a quite complex TS-integrated LSP server, so any insights you have on these would be very appreciated! Feel free to move this discussion to a TS issue if you'd prefer. Thank you! |
|
The two mechanisms look good. We can probably check whether the requested document's version matches the version in the TS snapshot. Then wait for the "snapshotUpdate" event when our version is newer. Speaking of document versions, I am also wondering whether unopened files (program files not opened in the editor) also need to have a version/hash? We might parse ts sourceFile's original Svelte content into svelte/html AST. I'm wondering if we can cache the AST result. Right now, it seems like we also cache the sourceFile and check if the new sourceFile is the same object. But I am not sure whether caching sourceFile leads to any memory leaks. Client middlewareThe problem with document-highlight is that VSCode only loads the first non-nullable result. Now that I've tried it again, it does allow multiple providers when I changed TypeScript's result to be null instead of an empty array. I probably did something wrong the last time I tried it. Is it possible for the TypeScript LSP to return Another place we might need a client middleware is completion. The problem is that we currently generate the default export as In general, I think I will try to see if we can just let VS Code merge the result from two servers. So most likely we will need middleware in features that uses the "first not empty/null" result, or if we need to tweak the result. |
…nto ts7-content-mapper
…nto ts7-content-mapper
|
A few content mapper tasks after this PR is merged:
LSP
Drop svelte-check's dependency on TypeScript 6
|
|
I tested the content mapper from d3766bc on a small fixture with Svelte 5.57.2, TypeScript 6.0.3, and With the mapper configured in node node_modules/@typescript/native/bin/tsc -p tsconfig.cm.json --noEmit --pretty false --runExternalCodeA After adding a string-to-number assignment error, the mapper reports TS2322 at These are CLI fixture checks; I haven't tested editor integration. |
|
One additional CLI repro with d3766bc, using the same setup as above: Svelte 5.57.2, TypeScript 6.0.3, and
<script lang="ts" generics="const T extends readonly string[]">
let { items }: { items: T } = $props();
</script>
<p>{items.length}</p>With the mapper configured in node node_modules/@typescript/native/bin/tsc -p tsconfig.cm.json --noEmit --pretty false --runExternalCodereports: The diagnostic notes that this is a virtual-code location with no corresponding source location. The generated type alias keeps type Gen__SvelteComponent_<const T extends readonly string[]> = InstanceType<typeof Gen__SvelteComponent_<T>>;TypeScript doesn't allow |
|
Another CLI finding with d3766bc: with the mapper, TypeScript type-checks the Minimal repro, with the mapper from d3766bc installed as in the PR description, Svelte 5.57.2, TypeScript 6.0.3 and
{
"compilerOptions": { "target": "ESNext", "module": "ESNext", "moduleResolution": "bundler", "strict": true, "noEmit": true, "skipLibCheck": true, "types": ["svelte"] },
"include": ["src/**/*.ts", "src/**/*.svelte"]
}
{
"extends": "./tsconfig.json",
"contentMappers": [{ "package": "@sveltejs/ts-content-mapper", "extensions": [".svelte"] }]
}
{
"name": "my-lib",
"version": "1.0.0",
"type": "module",
"exports": { ".": { "types": "./dist/index.d.ts", "svelte": "./dist/index.js" } }
}
declare const Button: import('svelte').Component<{ label: string }, {}, ''>;
type Button = ReturnType<typeof Button>;
export default Button;
<script lang="ts">
let { label }: { label: string } = $props();
const width: number = label;
</script>
<button style:width>{label}</button>
import { Button } from "my-lib";
export const button: typeof Button = Button;node node_modules/@typescript/native/bin/tsc -p tsconfig.json --noEmit --pretty false
node node_modules/@typescript/native/bin/tsc -p tsconfig.cm.json --noEmit --pretty false --runExternalCodeThe first passes. The second reports: svelte-check 4.7.7's default mode passes too. The resolver takes a fully specified import of a mapped extension as the file itself, before any declaration file: resolver.go#L1758-L1764, with I tried the resolver change this would need, on microsoft/TypeScript 98add1bbed (the 22 Oct nightly's commit), in Diff (+13/-6)diff --git a/tsc/internal/module/resolver.go b/tsc/internal/module/resolver.go
index 4b59f0582f..5a097b20d1 100644
--- a/tsc/internal/module/resolver.go
+++ b/tsc/internal/module/resolver.go
@@ -1756,15 +1756,22 @@ func (r *resolutionState) tryAddingExtensions(extensionless tspath.FileNameStem,
}
return continueSearching()
default:
- if slices.Contains(r.resolver.extraExtensions, originalExtension) {
- // A fully specified import of an extraExtension resolves directly to the file.
- if resolved := r.tryExtension(originalExtension, extensionless, false); !resolved.shouldContinueSearching() {
- resolved.resolvedUsingExtraExtensions = true
+ if extensions&extensionsDeclaration != 0 && !extensionless.AppendSuffix(originalExtension).IsDeclarationFile() {
+ if resolved := r.tryExtension(".d"+originalExtension+".ts", extensionless, false); !resolved.shouldContinueSearching() {
return resolved
}
}
- if extensions&extensionsDeclaration != 0 && !extensionless.AppendSuffix(originalExtension).IsDeclarationFile() {
- if resolved := r.tryExtension(".d"+originalExtension+".ts", extensionless, false); !resolved.shouldContinueSearching() {
+ if slices.Contains(r.resolver.extraExtensions, originalExtension) {
+ // A declaration file beside a mapped file describes it, as foo.d.ts describes foo.js:
+ // packages ship the mapped source their bundlers need next to its declarations.
+ if extensions&extensionsDeclaration != 0 {
+ if resolved := r.tryExtension(tspath.ExtensionDts, extensionless.AppendSuffix(originalExtension).AsStem(), false); !resolved.shouldContinueSearching() {
+ return resolved
+ }
+ }
+ // A fully specified import of an extraExtension resolves directly to the file.
+ if resolved := r.tryExtension(originalExtension, extensionless, false); !resolved.shouldContinueSearching() {
+ resolved.resolvedUsingExtraExtensions = true
return resolved
}
}Built with Whether mapped files should prefer their declarations everywhere, as |
|
In content mapper mode, TypeScript takes full control of module resolution logic. You can open an issue in TypeScript. That's indeed kind of a surprising result. For the |
|
For the module resolution problem. Now that I think of it. I guess that means that content mapper files are treated like a Although not ideal, it's also not a complete blocker. Component libraries can still rely on the package.json export-map to remap the typing to |
|
Ah. Wait. It's still a problem. Export map won't work for barrel export. |
|
We tried a few ways around the barrel case (7.1 nightly with the mapper at this PR's head,
Happy to share any of the patches if one is useful. |
#2733
The language server part currently requires changes in the TypeScript 7 VSCode extension that haven't been published yet. This can only be tested with a local TypeScript extension build.
This PR is focused on editor support. Svelte-check support will be a separate PR.
Known problems
Duplicated entries. Opened an upstream issue and also reduced the places where it happens.
VS Code doesn't seem to allow multiple providers. Need to manually merge it when we have a middleware api.
PR Test instruction
(Will edit this after this change is published)
svelte-check --tsgo-experimental-apiisn't compatiable with the latest nightly version.So you need to avoid workspace tsdk config.{ "devDependencies": { "@sveltejs/ts-content-mapper": "file:../language-tools/packages/typescript-content-mapper" }, "pnpm": { "overrides": { "@sveltejs/load-config": "file:../language-tools/packages/load-config", "svelte2tsx": "file:../language-tools/packages/svelte2tsx" } }