refactor(compiler-cli): abstract type check block metadata to be AST-free#67109
refactor(compiler-cli): abstract type check block metadata to be AST-free#67109atscott wants to merge 1 commit intoangular:mainfrom
Conversation
…free This commit refactors the template type checking metadata interfaces to use detached, serializable metadata rather than retaining direct references to ts.Node or ts.Declaration instances. A new tcb_adapter translates traditional TypeScript AST-bound metadata into these decoupled structures. This abstraction lays the groundwork for supporting native preprocessors (such as Rust or ts-go) which serialize metadata over JSON rather than passing live TypeScript objects. Key changes: - Introduced TcbDirectiveMetadata, TcbComponentMetadata, TcbReferenceMetadata, and TcbPipeMetadata to replace TypeCheckableDirectiveMeta where appropriate. - Substituted deep TS compilation AST references with string module names and source spans to preserve out-of-band diagnostic capabilities. - Detached generic typeParameters and transformType properties into synthesized, standalone TS mappings. - Updated generateTypeCheckBlock and corresponding Operations to consume the new metadata.
682e421 to
fc523b6
Compare
| if (this.typeCtors.has(node)) { | ||
| return this.typeCtors.get(node)!; | ||
| typeCtorFor(dir: TcbDirectiveMetadata): ts.Expression { | ||
| const key = dir.ref.moduleName ? `${dir.ref.moduleName}#${dir.ref.name}` : dir.ref.name; |
There was a problem hiding this comment.
Hmm, both moduleName and class names are not exactly unique. moduleName is also not defined for relative-path imports, so we risk colliding class names across multiple files.
A better option: key by absolute file path + '#' + position offset of the class in the source file. Much more reliable. We should extract that computation to a utility file/method and give it a branded type for safety.
| name = emitted.expression.value.name!; | ||
| moduleName = emitted.expression.value.moduleName; | ||
| isLocal = false; | ||
| } |
There was a problem hiding this comment.
Shouldn't we throw/fail if emitted.kind is not Success?
There was a problem hiding this comment.
This is due to computing this up-front. In the previous code, env.referenceType(this.dir.ref) or env.referenceExternalType(this.dir.ref) happened inside operations and would only happen if that specific component's TCB evaluation reached a point where the reference was required.
Now we are translating everything up front to strip TS AST references.
| let type: ts.TypeNode; | ||
| let span: ParseSourceSpan; | ||
| if (this.dir.isGeneric === false || dirRef.node.typeParameters === undefined) { | ||
| if (this.dir.isGeneric === false || this.dir.typeParameters?.length === 0) { |
There was a problem hiding this comment.
This will fail the condition if this.dir.typeParameters is undefined. It needs to be (this.dir.typeParameters?.length ?? 0) === 0. Better yet, make typeParameters required (I almost always avoid optional members of metadata objects for this reason).
There was a problem hiding this comment.
oops, yea I was trying to revert these changes but missed one...
| meta: TypeCheckBlockMetadata, | ||
| env: Environment, | ||
| ): {tcbMeta: TcbTypeCheckBlockMetadata; component: TcbComponentMetadata} { | ||
| const dirCache = new Map<any, TcbDirectiveMetadata>(); |
| } | ||
| return target as ReferenceTarget<TcbDirectiveMetadata>; | ||
| }, | ||
| getDeferredTriggerTarget: (b: any, t: any) => meta.boundTarget.getDeferredTriggerTarget(b, t), |
| * provided. | ||
| */ | ||
| referenceExternalType(moduleName: string, name: string, typeParams?: Type[]): ts.TypeNode { | ||
| referenceExternalType(moduleName: string | null, name: string, typeParams?: Type[]): ts.TypeNode { |
There was a problem hiding this comment.
The | null is odd here - external types should always have a module name we're trying to reference.
This commit refactors the template type checking metadata interfaces to use detached, serializable metadata rather than retaining direct references to ts.Node or ts.Declaration instances.
A new tcb_adapter translates traditional TypeScript AST-bound metadata into these decoupled structures. This abstraction lays the groundwork for supporting native preprocessors (such as Rust or ts-go) which serialize metadata over JSON rather than passing live TypeScript objects.
Key changes: