feat(ts-content-mapper): add a TypeScript content mapper for .astro files - #17835
Conversation
|
21c2668 to
796841a
Compare
47915e9 to
e6e7a45
Compare
| export type PositionEncoding = 'utf-8' | 'utf-16'; | ||
|
|
||
| export interface InitializeParams { | ||
| positionEncodings: PositionEncoding[]; | ||
| } | ||
|
|
||
| export interface InitializeResult { | ||
| positionEncoding: PositionEncoding; | ||
| diagnosticSource: string; | ||
| } | ||
|
|
||
| export interface TransformParams { | ||
| fileName: string; | ||
| content: string; | ||
| projectHandle: string; | ||
| } | ||
|
|
||
| export interface SupplementalOutput { | ||
| text: string; | ||
| extension: '.mjs' | '.mts'; | ||
| mappings: ConvertToTsxResult['mappings']; | ||
| } | ||
|
|
||
| export interface TransformResult { | ||
| text: string; | ||
| extension: '.tsx'; | ||
| mappings: ConvertToTsxResult['mappings']; | ||
| diagnostics: MapperDiagnostic[]; | ||
| supplemental?: SupplementalOutput[]; | ||
| } | ||
|
|
||
| export interface MapperDiagnostic { | ||
| messageText: string; | ||
| start: number; | ||
| length: number; | ||
| code: number; | ||
| } |
There was a problem hiding this comment.
This is a bunch of types that I feel should be exported from TypeScript directly, but I couldn't find them and anyway, with it being just in the nightly for now we can't really depend on it even if we wanted to.
There was a problem hiding this comment.
Would it possible to share the feedback with the TS team to see if we could depend on them once shipped?
There was a problem hiding this comment.
Looking around it seems like this is already planned, it's just currently in some unstable modules.
| import type { InitializeParams, InitializeResult } from './protocol.js'; | ||
| import { transform } from './transform.js'; | ||
|
|
||
| const POSITION_ENCODING = 'utf-16'; |
There was a problem hiding this comment.
| @@ -0,0 +1,27 @@ | |||
| # @astrojs/ts-content-mapper | |||
|
|
|||
| A [TypeScript content mapper](https://github.com/microsoft/TypeScript/pull/63936) for `.astro` files. It lets TypeScript 7.1+ (`tsc`) parse and type-check Astro components directly, without needing to use a different language server or CLI Tool. | |||
There was a problem hiding this comment.
I don't think there's a better link for now unfortunately, when 7.1 releases I assume the blog post will talk about it.
863278c to
7f363d4
Compare
| (script) => | ||
| script.type === ExtractedScriptType.Inline || | ||
| script.type === ExtractedScriptType.EventAttribute || | ||
| script.type === ExtractedScriptType.Unknown, |
There was a problem hiding this comment.
what is an unknown script?
There was a problem hiding this comment.
It's when we don't know what's inside a script, like <script type="something-unknown"></script> or <script type={dynamic}>. Fwiw this code is the same as the previous implementation with the same enums etc.
|
|
||
| const VERBATIM = 0; | ||
| const ATOM = 1; | ||
| const DEFINITION = 1 << 3; |
matthewp
left a comment
There was a problem hiding this comment.
less code than I expected
Changes
This PR adds the long-awaited TypeScript 7 content mapper for Astro, waouw!
Users of TS >=7.1 are able to install this package, configure it in their
tsconfig.json, runtscnormally (well, with a flag for now) and get their.astrofiles type checked as if they were just normal TS files. This also support emitting and editor features.For the editor part, there's some work still needed in the language server and VS Code extension to make it register the content mapper for users automatically and not have both the old pipeline and new one running at once, see #17852
Testing
Added tests for the mappings existing and being generated correctly.
We unfortunately can't add full E2E tests for this until TypeScript 7.1 is fully out and we're migrated because otherwise you get multiple TS versions and everything breaks, or at least that was my experience. Couldn't figure it out.
Documentation
Probably needs a page or mention somewhere, but since this is only for nightly right now, I don't think it's required for this to get merged