Repository navigation
Conversation
f415220 to
228c153
Compare
Transform Svelte components with svelte2tsx while mapping symbols and ranges back to the original source. Cover runes, legacy and JavaScript components, imported stores, malformed files, modern module resolution, and cross-project prop identity. Amp-Thread-ID: https://ampcode.com/threads/T-01a029e2-5095-710a-9cce-0c6d8645c173
Limit direct import-alias resolution to generated Svelte sources and imports from .svelte modules so ordinary TypeScript indexes remain byte-for-byte unchanged. Amp-Thread-ID: https://ampcode.com/threads/T-01a029e2-5095-710a-9cce-0c6d8645c173
Amp-Thread-ID: https://ampcode.com/threads/T-01a0f298-28ec-7699-bc15-3ac5b66731e5 Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-01a1595a-e083-7682-965e-47f2c8c887d6 Co-authored-by: Amp <amp@ampcode.com>
228c153 to
127621d
Compare
christoph-sg
left a comment
christoph-sg
left a comment
There was a problem hiding this comment.
I have a few points worth discussing a bit. I think in general this looks quite nice, and as long as we make sure the svelte specific code is guarded by the svelte flag, it should be low-risk to include.
| if (contextualType === undefined) { | ||
| return | ||
| } | ||
| // svelte2tsx gives the generated props object the contextual type |
There was a problem hiding this comment.
I think I'd like to pass down a flag isSvelte into the FileIndexer, and then guard these blocks (maybe move them into separate functions). Just so we have a stronger indicator than comments (that rot as soon as you commit them), that there's something Svelte specific going on.
Looks like a fair amount of those Svelte specific functions already live in Svelte.ts, that might be a good place.
| // Ambient declarations for the __sveltets helpers emitted by | ||
| // svelte2tsx. They let TypeScript infer component props and generics, | ||
| // but are not in config.fileNames and therefore are not indexed. | ||
| require.resolve('svelte2tsx/svelte-shims-v4.d.ts'), |
There was a problem hiding this comment.
How quickly is this v4 going to be out of date? Can we do something more durable? I'm also a bit worried this might be relying too much on Svelte internals, I don't think they'll consider the layout of their generated code as public API.
| occurrences: [], | ||
| }) | ||
| const input = new Input(sourceFile.fileName, sourceFile.getText()) | ||
| const input = new Input(sourceInfo.fileName, sourceInfo.text) |
There was a problem hiding this comment.
This .text vs .getText() seems to be popping up over and over, and flip-flopping. Is there one "good" way of doing it, and would it be worth trying to enforce with something like eslint?
| this.host, | ||
| this.compilerOptions | ||
| ) | ||
| this.sourceInfos.set( |
There was a problem hiding this comment.
Does this mean we hold onto the compiled output for every svelte file in the project throughout the indexing process? That might be fine, but if we could free it after indexing each individual file, we might be able to reduce max-memory use substantially.
2 participants
This PR implements basic support for indexing Svelte projects with this indexer as well. It uses
svelte2tsxand maps SCIP symbols and ranges back to the original.sveltesource files via the source maps.Verified against the Sourcegraph webapp, everything passes.
See the included snapshots for some example data.
This is likely not 100% perfect, but it's a solid start that we can build on more (just like scip-typescript itself).