Skip to content

Commit 2eae497

Browse files
atscottamishne
atscott
authored and
amishne
committed
feat(compiler-cli): support external TCBs with copied content in specific mode
 This change adds a new that allows environments that cannot support inline TCBs (such as the language service or source-to-source transforms where TS compilation and emit are downstream) to still perform template type checking. Instead of inlining the TCB into the original source file when non-exported symbols are referenced, we now copy the file content to the .ngtypecheck.ts shim file and generate the external TCB there, if requested by the inlining mode. This preserves the local scope of the original file while keeping the original file unmodified.
1 parent 12d4844 commit 2eae497

13 files changed

Lines changed: 237 additions & 68 deletions

File tree

‎packages/compiler-cli/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ export {
6161
} from './src/ngtsc/reflection';
6262
export {isFatalDiagnosticError} from './src/ngtsc/diagnostics';
6363
export {PerfPhase} from './src/ngtsc/perf';
64-
export {type FileUpdate, type ProgramDriver} from './src/ngtsc/program_driver';
64+
export {type FileUpdate, InliningMode, type ProgramDriver} from './src/ngtsc/program_driver';
6565
export {TrackedIncrementalBuildStrategy} from './src/ngtsc/incremental';
6666
export {isShim} from './src/ngtsc/shims';
6767
export {getRootDirs} from './src/ngtsc/util/src/typescript';

‎packages/compiler-cli/src/ngtsc/core/src/compiler.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ import {
8787
PerfEvent,
8888
PerfPhase,
8989
} from '../../perf';
90-
import {FileUpdate, ProgramDriver, UpdateMode} from '../../program_driver';
90+
import {FileUpdate, InliningMode, ProgramDriver, UpdateMode} from '../../program_driver';
9191
import {DeclarationNode, isNamedClassDeclaration, TypeScriptReflectionHost} from '../../reflection';
9292
import {AdapterResourceLoader} from '../../resource';
9393
import {
@@ -1882,6 +1882,10 @@ class NotifyingProgramDriverWrapper implements ProgramDriver {
18821882
this.getSourceFileVersion = this.delegate.getSourceFileVersion?.bind(this);
18831883
}
18841884

1885+
get inliningMode(): InliningMode {
1886+
return this.delegate.inliningMode;
1887+
}
1888+
18851889
get supportsInlineOperations() {
18861890
return this.delegate.supportsInlineOperations;
18871891
}

‎packages/compiler-cli/src/ngtsc/program_driver/src/api.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,21 @@ export interface MaybeSourceFileWithOriginalFile extends ts.SourceFile {
3232
[NgOriginalFile]?: ts.SourceFile;
3333
}
3434

35+
/**
36+
* How a type-checking context should handle operations which would require inlining.
37+
*/
38+
export enum InliningMode {
39+
InlineOps,
40+
Error,
41+
CopySourceToTcb,
42+
}
43+
3544
export interface ProgramDriver {
45+
/**
46+
* How this strategy handles operations that require inlining.
47+
*/
48+
readonly inliningMode: InliningMode;
49+
3650
/**
3751
* Whether this strategy supports modifying user files (inline modifications) in addition to
3852
* modifying type-checking shims.

‎packages/compiler-cli/src/ngtsc/program_driver/src/ts_create_program_driver.ts‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {RequiredDelegations, toUnredirectedSourceFile} from '../../util/src/type
1414

1515
import {
1616
FileUpdate,
17+
InliningMode,
1718
MaybeSourceFileWithOriginalFile,
1819
NgOriginalFile,
1920
ProgramDriver,
@@ -27,10 +28,10 @@ import {
2728
* If a new method is added to `ts.CompilerHost` which is not delegated, a type error will be
2829
* generated for this class.
2930
*/
30-
export class DelegatingCompilerHost
31-
implements
32-
Omit<RequiredDelegations<ts.CompilerHost>, 'getSourceFile' | 'fileExists' | 'writeFile'>
33-
{
31+
export class DelegatingCompilerHost implements Omit<
32+
RequiredDelegations<ts.CompilerHost>,
33+
'getSourceFile' | 'fileExists' | 'writeFile'
34+
> {
3435
createHash;
3536
directoryExists;
3637
getCancellationToken;
@@ -212,7 +213,14 @@ export class TsCreateProgramDriver implements ProgramDriver {
212213
this.program = this.originalProgram;
213214
}
214215

215-
readonly supportsInlineOperations = true;
216+
inliningMode = InliningMode.InlineOps;
217+
218+
get supportsInlineOperations(): boolean {
219+
return (
220+
this.inliningMode === InliningMode.InlineOps ||
221+
this.inliningMode === InliningMode.CopySourceToTcb
222+
);
223+
}
216224

217225
getProgram(): ts.Program {
218226
return this.program;

‎packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,6 @@ import {makeTemplateDiagnostic} from '../diagnostics';
107107

108108
import {CompletionEngine} from './completion';
109109
import {
110-
InliningMode,
111110
ShimTypeCheckingData,
112111
TypeCheckData,
113112
TypeCheckContextImpl,
@@ -627,6 +626,9 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
627626
}
628627

629628
for (const [shimPath, shimRecord] of fileRecord.shimData) {
629+
// TODO(atscott): Filter out diagnostics from original source in CopySourceToTcb
630+
// We don't want to duplicate diagnostics from original source when copying to tcb
631+
630632
const shimSf = getSourceFileOrError(typeCheckProgram, shimPath);
631633
diagnostics.push(
632634
...typeCheckProgram
@@ -993,16 +995,13 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
993995
}
994996

995997
private newContext(host: TypeCheckingHost): TypeCheckContextImpl {
996-
const inlining = this.programDriver.supportsInlineOperations
997-
? InliningMode.InlineOps
998-
: InliningMode.Error;
999998
return new TypeCheckContextImpl(
1000999
this.config,
10011000
this.compilerHost,
10021001
this.refEmitter,
10031002
this.reflector,
10041003
host,
1005-
inlining,
1004+
this.programDriver.inliningMode,
10061005
this.perf,
10071006
);
10081007
}

‎packages/compiler-cli/src/ngtsc/typecheck/src/context.ts‎

Lines changed: 74 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import {ErrorCode, makeDiagnostic, ngErrorCode} from '../../../../src/ngtsc/diag
3030
import {absoluteFromSourceFile, AbsoluteFsPath} from '../../file_system';
3131
import {Reference, ReferenceEmitter} from '../../imports';
3232
import {PerfEvent, PerfRecorder} from '../../perf';
33-
import {FileUpdate} from '../../program_driver';
33+
import {FileUpdate, InliningMode} from '../../program_driver';
3434
import {ClassDeclaration, ReflectionHost} from '../../reflection';
3535
import {ImportManager} from '../../translator';
3636
import {
@@ -125,6 +125,11 @@ export interface PendingFileTypeCheckingData {
125125
* Map of in-progress shim data for shims generated from this input file.
126126
*/
127127
shimData: Map<AbsoluteFsPath, PendingShimData>;
128+
129+
/**
130+
* The original source file.
131+
*/
132+
sourceFile?: ts.SourceFile;
128133
}
129134

130135
export interface PendingShimData {
@@ -188,21 +193,6 @@ export interface TypeCheckingHost {
188193
recordComplete(sfPath: AbsoluteFsPath): void;
189194
}
190195

191-
/**
192-
* How a type-checking context should handle operations which would require inlining.
193-
*/
194-
export enum InliningMode {
195-
/**
196-
* Use inlining operations when required.
197-
*/
198-
InlineOps,
199-
200-
/**
201-
* Produce diagnostics if an operation would require inlining.
202-
*/
203-
Error,
204-
}
205-
206196
/**
207197
* A template type checking context for a program.
208198
*
@@ -391,11 +381,16 @@ export class TypeCheckContextImpl implements TypeCheckContext {
391381
this.perf.eventCount(PerfEvent.GenerateTcb);
392382
if (
393383
inliningRequirement !== TcbInliningRequirement.None &&
394-
this.inlining === InliningMode.InlineOps
384+
(this.inlining === InliningMode.InlineOps || this.inlining === InliningMode.CopySourceToTcb)
395385
) {
396-
// This class didn't meet the requirements for external type checking, so generate an inline
397-
// TCB for the class.
386+
// Queue operations for both inline and copy strategy!
387+
// The decision on where to apply them will be made in finalize().
398388
this.addInlineTypeCheckBlock(fileData, shimData, ref, meta);
389+
390+
if (this.inlining === InliningMode.CopySourceToTcb) {
391+
// Still set the original file path to force local references for symbols from this file.
392+
shimData.file.copiedSourceOriginPath = absoluteFromSourceFile(sourceFile);
393+
}
399394
} else if (
400395
inliningRequirement === TcbInliningRequirement.ShouldInlineForGenericBounds &&
401396
this.inlining === InliningMode.Error
@@ -451,18 +446,9 @@ export class TypeCheckContextImpl implements TypeCheckContext {
451446
}
452447

453448
/**
454-
* Transform a `ts.SourceFile` into a version that includes type checking code.
455-
*
456-
* If this particular `ts.SourceFile` requires changes, the text representing its new contents
457-
* will be returned. Otherwise, a `null` return indicates no changes were necessary.
449+
* Applies operations to a file.
458450
*/
459-
transform(sf: ts.SourceFile): string | null {
460-
// If there are no operations pending for this particular file, return `null` to indicate no
461-
// changes.
462-
if (!this.opMap.has(sf)) {
463-
return null;
464-
}
465-
451+
private executeOperations(targetSf: ts.SourceFile, opsSourceSf: ts.SourceFile): string {
466452
// Use a `ts.Printer` to generate source code.
467453
const printer = ts.createPrinter({omitTrailingSemicolon: true});
468454

@@ -479,39 +465,43 @@ export class TypeCheckContextImpl implements TypeCheckContext {
479465
// Execute ops.
480466
// Each Op has a splitPoint index into the text where it needs to be inserted.
481467
const updates: {pos: number; deletePos?: number; text: string}[] = this.opMap
482-
.get(sf)!
468+
.get(opsSourceSf)!
483469
.map((op) => {
484470
return {
485471
pos: op.splitPoint,
486-
text: op.execute(importManager, sf, this.refEmitter),
472+
text: op.execute(importManager, targetSf, this.refEmitter),
487473
};
488474
});
489475

490476
const {newImports, updatedImports} = importManager.finalize();
491477

492478
// Capture new imports
493-
if (newImports.has(sf.fileName)) {
494-
newImports.get(sf.fileName)!.forEach((newImport) => {
479+
if (newImports.has(targetSf.fileName)) {
480+
newImports.get(targetSf.fileName)!.forEach((newImport) => {
495481
updates.push({
496482
pos: 0,
497-
text: printer.printNode(ts.EmitHint.Unspecified, newImport, sf),
483+
text: printer.printNode(ts.EmitHint.Unspecified, newImport, targetSf),
498484
});
499485
});
500486
}
501487

502488
// Capture updated imports
503489
for (const [oldBindings, newBindings] of updatedImports.entries()) {
504-
if (oldBindings.getSourceFile() !== sf) {
490+
if (oldBindings.getSourceFile() !== targetSf) {
505491
throw new Error('Unexpected updates to unrelated source files.');
506492
}
507493
updates.push({
508494
pos: oldBindings.getStart(),
509495
deletePos: oldBindings.getEnd(),
510-
text: printer.printNode(ts.EmitHint.Unspecified, newBindings, sf),
496+
text: printer.printNode(ts.EmitHint.Unspecified, newBindings, targetSf),
511497
});
512498
}
513499

514-
const result = new MagicString(sf.text, {filename: sf.fileName});
500+
// TODO: Consider generating a sourcemap here via `result.generateMap()`.
501+
// This could be used in `CopySourceToTcb` mode to map positions in the shim file
502+
// back to the original source file, helping with language features like "Go to Definition"
503+
// and diagnostic translation.
504+
const result = new MagicString(targetSf.text, {filename: targetSf.fileName});
515505
for (const update of updates) {
516506
if (update.deletePos !== undefined) {
517507
result.remove(update.pos, update.deletePos);
@@ -521,11 +511,35 @@ export class TypeCheckContextImpl implements TypeCheckContext {
521511
return result.toString();
522512
}
523513

514+
/**
515+
* Generates the transformed text for an original source file.
516+
*/
517+
private generateTransformedOriginalFile(sf: ts.SourceFile): string | null {
518+
if (this.inlining !== InliningMode.InlineOps || !this.opMap.has(sf)) {
519+
return null;
520+
}
521+
return this.executeOperations(sf, sf);
522+
}
523+
524+
/**
525+
* Generates the content for a shim file that copies the source of the original file.
526+
*/
527+
private generateCopiedShimContent(
528+
originalSf: ts.SourceFile,
529+
shimFileName: string,
530+
): string | null {
531+
if (this.inlining !== InliningMode.CopySourceToTcb || !this.opMap.has(originalSf)) {
532+
return null;
533+
}
534+
const fakeSf = ts.createSourceFile(shimFileName, originalSf.text, ts.ScriptTarget.Latest, true);
535+
return this.executeOperations(fakeSf, originalSf);
536+
}
537+
524538
finalize(): Map<AbsoluteFsPath, FileUpdate> {
525539
// First, build the map of updates to source files.
526540
const updates = new Map<AbsoluteFsPath, FileUpdate>();
527541
for (const originalSf of this.opMap.keys()) {
528-
const newText = this.transform(originalSf);
542+
const newText = this.generateTransformedOriginalFile(originalSf);
529543
if (newText !== null) {
530544
updates.set(absoluteFromSourceFile(originalSf), {
531545
newText,
@@ -553,6 +567,19 @@ export class TypeCheckContextImpl implements TypeCheckContext {
553567
path: pendingShimData.file.fileName,
554568
data: pendingShimData.data,
555569
});
570+
571+
// Set the source content on the shim file before rendering!
572+
const originalSf = pendingFileData.sourceFile;
573+
if (originalSf !== undefined) {
574+
const transformedText = this.generateCopiedShimContent(
575+
originalSf,
576+
pendingShimData.file.fileName,
577+
);
578+
if (transformedText !== null) {
579+
pendingShimData.file.setSourceContent(transformedText);
580+
}
581+
}
582+
556583
const sfText = pendingShimData.file.render();
557584
updates.set(pendingShimData.file.fileName, {
558585
newText: sfText,
@@ -587,6 +614,7 @@ export class TypeCheckContextImpl implements TypeCheckContext {
587614
shimData.oobRecorder,
588615
),
589616
);
617+
590618
fileData.hasInlines = true;
591619
}
592620

@@ -615,6 +643,7 @@ export class TypeCheckContextImpl implements TypeCheckContext {
615643
hasInlines: false,
616644
sourceManager: this.host.getSourceManager(sfPath),
617645
shimData: new Map(),
646+
sourceFile: sf,
618647
};
619648
this.fileMap.set(sfPath, data);
620649
}
@@ -690,8 +719,12 @@ class InlineTcbOp implements Op {
690719
return this.ref.node.end + 1;
691720
}
692721

693-
execute(im: ImportManager, sf: ts.SourceFile, refEmitter: ReferenceEmitter): string {
694-
const env = new Environment(this.config, im, refEmitter, sf);
722+
execute(im: ImportManager, tcbSf: ts.SourceFile, refEmitter: ReferenceEmitter): string {
723+
const env = new Environment(this.config, im, refEmitter, tcbSf);
724+
const originalSf = this.ref.node.getSourceFile();
725+
if (tcbSf !== originalSf) {
726+
env.copiedSourceOriginPath = absoluteFromSourceFile(originalSf);
727+
}
695728
const fnName = `_tcb_${this.ref.node.pos}`;
696729

697730
const {tcbMeta, component} = adaptTypeCheckBlockMetadata(

‎packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
TypeCtorMetadata,
1616
} from '@angular/compiler';
1717
import ts from 'typescript';
18+
import {AbsoluteFsPath} from '../../file_system';
1819

1920
import {ReferenceEmitter} from '../../imports';
2021

@@ -46,13 +47,17 @@ export class Environment extends ReferenceEmitEnvironment {
4647
private pipeInsts = new Map<TcbReferenceKey, string>();
4748
protected pipeInstStatements: TcbExpr[] = [];
4849

50+
copiedSourceOriginPath?: AbsoluteFsPath;
51+
4952
constructor(
5053
readonly config: TypeCheckingConfig,
5154
importManager: ImportManager,
5255
refEmitter: ReferenceEmitter,
5356
contextFile: ts.SourceFile,
57+
copiedSourceOriginPath?: AbsoluteFsPath,
5458
) {
5559
super(importManager, refEmitter, contextFile);
60+
this.copiedSourceOriginPath = copiedSourceOriginPath;
5661
}
5762

5863
/**

0 commit comments

Comments
 (0)