diff --git a/package.json b/package.json index 44cc30765e7c1..c85d821597d27 100644 --- a/package.json +++ b/package.json @@ -118,7 +118,7 @@ "karma-jasmine": "^5.0.0", "karma-requirejs": "^1.1.0", "karma-sourcemap-loader": "^0.4.0", - "magic-string": "0.30.7", + "magic-string": "^0.30.8", "memo-decorator": "^2.0.1", "ngx-flamegraph": "0.0.12", "ngx-progressbar": "^11.1.0", diff --git a/packages/compiler-cli/src/ngtsc/typecheck/BUILD.bazel b/packages/compiler-cli/src/ngtsc/typecheck/BUILD.bazel index 47a64194b22dd..e070f16a35fc2 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/typecheck/BUILD.bazel @@ -26,6 +26,7 @@ ts_library( "//packages/compiler-cli/src/ngtsc/typecheck/diagnostics", "//packages/compiler-cli/src/ngtsc/util", "@npm//@types/node", + "@npm//magic-string", "@npm//typescript", ], ) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts index c216b3e520403..5f947c5843ac2 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts @@ -7,16 +7,17 @@ */ import {BoundTarget, ParseError, ParseSourceFile, R3TargetBinder, SchemaMetadata, TmplAstNode} from '@angular/compiler'; +import MagicString from 'magic-string'; import ts from 'typescript'; import {ErrorCode, ngErrorCode} from '../../../../src/ngtsc/diagnostics'; import {absoluteFromSourceFile, AbsoluteFsPath} from '../../file_system'; -import {NoopImportRewriter, Reference, ReferenceEmitter} from '../../imports'; +import {Reference, ReferenceEmitter} from '../../imports'; import {PipeMeta} from '../../metadata'; import {PerfEvent, PerfRecorder} from '../../perf'; import {FileUpdate} from '../../program_driver'; import {ClassDeclaration, ReflectionHost} from '../../reflection'; -import {ImportManager} from '../../translator'; +import {ImportManagerV2} from '../../translator'; import {TemplateDiagnostic, TemplateId, TemplateSourceMapping, TypeCheckableDirectiveMeta, TypeCheckBlockMetadata, TypeCheckContext, TypeCheckingConfig, TypeCtorMetadata} from '../api'; import {makeTemplateDiagnostic} from '../diagnostics'; @@ -27,7 +28,6 @@ import {ReferenceEmitEnvironment} from './reference_emit_environment'; import {TypeCheckShimGenerator} from './shim'; import {TemplateSourceManager} from './source'; import {requiresInlineTypeCheckBlock, TcbInliningRequirement} from './tcb_util'; -import {getImportString} from './ts_util'; import {generateTypeCheckBlock, TcbGenericContextBehavior} from './type_check_block'; import {TypeCheckFile} from './type_check_file'; import {generateInlineTypeCtor, requiresInlineTypeCtor} from './type_constructor'; @@ -358,34 +358,61 @@ export class TypeCheckContextImpl implements TypeCheckContext { return null; } - // Imports may need to be added to the file to support type-checking of directives used in the - // template within it. - const importManager = new ImportManager(new NoopImportRewriter(), '_i'); - - // Each Op has a splitPoint index into the text where it needs to be inserted. Split the - // original source text into chunks at these split points, where code will be inserted between - // the chunks. - const ops = this.opMap.get(sf)!.sort(orderOps); - const textParts = splitStringAtPoints(sf.text, ops.map(op => op.splitPoint)); - // Use a `ts.Printer` to generate source code. const printer = ts.createPrinter({omitTrailingSemicolon: true}); - // Begin with the initial section of the code text. - let code = textParts[0]; - - // Process each operation and use the printer to generate source code for it, inserting it into - // the source code in between the original chunks. - ops.forEach((op, idx) => { - const text = op.execute(importManager, sf, this.refEmitter, printer); - code += '\n\n' + text + textParts[idx + 1]; + // Imports may need to be added to the file to support type-checking of directives + // used in the template within it. + const importManager = new ImportManagerV2({ + // This minimizes noticeable changes with older versions of `ImportManager`. + forceGenerateNamespacesForNewImports: true, + // Type check block code affects code completion and fix suggestions. + // We want to encourage single quotes for now, like we always did. + shouldUseSingleQuotes: () => true, }); - // Write out the imports that need to be added to the beginning of the file. - let imports = importManager.getAllImports(sf.fileName).map(getImportString).join('\n'); - code = imports + '\n' + code; + // Execute ops. + // Each Op has a splitPoint index into the text where it needs to be inserted. + const updates: {pos: number, deletePos?: number, text: string}[] = + this.opMap.get(sf)!.map(op => { + return { + pos: op.splitPoint, + text: op.execute(importManager, sf, this.refEmitter, printer), + }; + }); + + const {newImports, updatedImports} = importManager.finalize(); + + // Capture new imports + if (newImports.has(sf.fileName)) { + newImports.get(sf.fileName)!.forEach(newImport => { + updates.push({ + pos: 0, + text: printer.printNode(ts.EmitHint.Unspecified, newImport, sf), + }); + }); + } + + // Capture updated imports + for (const [oldBindings, newBindings] of updatedImports.entries()) { + if (oldBindings.getSourceFile() !== sf) { + throw new Error('Unexpected updates to unrelated source files.'); + } + updates.push({ + pos: oldBindings.getStart(), + deletePos: oldBindings.getEnd(), + text: printer.printNode(ts.EmitHint.Unspecified, newBindings, sf), + }); + } - return code; + const result = new MagicString(sf.text, {filename: sf.fileName}); + for (const update of updates) { + if (update.deletePos !== undefined) { + result.remove(update.pos, update.deletePos); + } + result.appendLeft(update.pos, update.text); + } + return result.toString(); } finalize(): Map { @@ -510,8 +537,9 @@ interface Op { /** * Execute the operation and return the generated code as text. */ - execute(im: ImportManager, sf: ts.SourceFile, refEmitter: ReferenceEmitter, printer: ts.Printer): - string; + execute( + im: ImportManagerV2, sf: ts.SourceFile, refEmitter: ReferenceEmitter, + printer: ts.Printer): string; } /** @@ -531,16 +559,18 @@ class InlineTcbOp implements Op { return this.ref.node.end + 1; } - execute(im: ImportManager, sf: ts.SourceFile, refEmitter: ReferenceEmitter, printer: ts.Printer): - string { + execute( + im: ImportManagerV2, sf: ts.SourceFile, refEmitter: ReferenceEmitter, + printer: ts.Printer): string { const env = new Environment(this.config, im, refEmitter, this.reflector, sf); const fnName = ts.factory.createIdentifier(`_tcb_${this.ref.node.pos}`); - // Inline TCBs should copy any generic type parameter nodes directly, as the TCB code is inlined - // into the class in a context where that will always be legal. + // Inline TCBs should copy any generic type parameter nodes directly, as the TCB code is + // inlined into the class in a context where that will always be legal. const fn = generateTypeCheckBlock( env, this.ref, fnName, this.meta, this.domSchemaChecker, this.oobRecorder, TcbGenericContextBehavior.CopyClassNodes); + return printer.printNode(ts.EmitHint.Unspecified, fn, sf); } } @@ -560,8 +590,9 @@ class TypeCtorOp implements Op { return this.ref.node.end - 1; } - execute(im: ImportManager, sf: ts.SourceFile, refEmitter: ReferenceEmitter, printer: ts.Printer): - string { + execute( + im: ImportManagerV2, sf: ts.SourceFile, refEmitter: ReferenceEmitter, + printer: ts.Printer): string { const emitEnv = new ReferenceEmitEnvironment(im, refEmitter, this.reflector, sf); const tcb = generateInlineTypeCtor(emitEnv, this.ref.node, this.meta); return printer.printNode(ts.EmitHint.Unspecified, tcb, sf); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts index d1b184d49bcb2..ce5acb42e1d09 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts @@ -10,7 +10,7 @@ import ts from 'typescript'; import {assertSuccessfulReferenceEmit, ImportFlags, Reference, ReferenceEmitter} from '../../imports'; import {ClassDeclaration, ReflectionHost} from '../../reflection'; -import {ImportManager, translateExpression} from '../../translator'; +import {ImportManagerV2, translateExpression} from '../../translator'; import {TypeCheckableDirectiveMeta, TypeCheckingConfig, TypeCtorMetadata} from '../api'; import {ReferenceEmitEnvironment} from './reference_emit_environment'; @@ -42,7 +42,7 @@ export class Environment extends ReferenceEmitEnvironment { protected pipeInstStatements: ts.Statement[] = []; constructor( - readonly config: TypeCheckingConfig, importManager: ImportManager, + readonly config: TypeCheckingConfig, importManager: ImportManagerV2, refEmitter: ReferenceEmitter, reflector: ReflectionHost, contextFile: ts.SourceFile) { super(importManager, refEmitter, reflector, contextFile); } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/reference_emit_environment.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/reference_emit_environment.ts index 1a70753c2883c..45d984a39079c 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/reference_emit_environment.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/reference_emit_environment.ts @@ -11,7 +11,7 @@ import ts from 'typescript'; import {assertSuccessfulReferenceEmit, ImportFlags, Reference, ReferenceEmitKind, ReferenceEmitter} from '../../imports'; import {ReflectionHost} from '../../reflection'; -import {ImportManager, translateExpression, translateType} from '../../translator'; +import {ImportManagerV2, translateExpression, translateType} from '../../translator'; /** * An environment for a given source file that can be used to emit references. @@ -21,7 +21,7 @@ import {ImportManager, translateExpression, translateType} from '../../translato */ export class ReferenceEmitEnvironment { constructor( - readonly importManager: ImportManager, protected refEmitter: ReferenceEmitter, + readonly importManager: ImportManagerV2, protected refEmitter: ReferenceEmitter, readonly reflector: ReflectionHost, public contextFile: ts.SourceFile) {} canReferenceType( diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/tcb_util.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/tcb_util.ts index 07100b3ac3302..12814821dc28b 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/tcb_util.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/tcb_util.ts @@ -19,17 +19,21 @@ import {ReferenceEmitEnvironment} from './reference_emit_environment'; import {TypeParameterEmitter} from './type_parameter_emitter'; /** - * External modules that always should exist for type check blocks and - * file hosting inline type constructors. + * External modules/identifiers that always should exist for type check + * block files. * * Importing the modules in preparation helps ensuring a stable import graph * that would not degrade TypeScript's incremental program structure re-use. + * + * Note: For inline type check blocks, or type constructors, we cannot add preparation + * imports, but ideally the required modules are already imported and can be re-used + * to not incur a structural TypeScript program re-use discarding. */ -const TCB_FILE_IMPORT_GRAPH_PREPARE_MODULES = [ +const TCB_FILE_IMPORT_GRAPH_PREPARE_IDENTIFIERS = [ // Imports may be added for signal input checking. We wouldn't want to change the // import graph for incremental compilations when suddenly a signal input is used, // or removed. - R3Identifiers.InputSignalBrandWriteType.moduleName, + R3Identifiers.InputSignalBrandWriteType, ]; /** @@ -194,8 +198,12 @@ function getTemplateId( * import graph changes whenever e.g. a signal input is introduced in user code. */ export function ensureTypeCheckFilePreparationImports(env: ReferenceEmitEnvironment): void { - for (const moduleName of TCB_FILE_IMPORT_GRAPH_PREPARE_MODULES) { - env.importManager.generateNamespaceImport(moduleName); + for (const identifier of TCB_FILE_IMPORT_GRAPH_PREPARE_IDENTIFIERS) { + env.importManager.addImport({ + exportModuleSpecifier: identifier.moduleName, + exportSymbolName: identifier.name, + requestedFile: env.contextFile, + }); } } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/ts_util.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/ts_util.ts index a943050d8a16f..33aee30fb4112 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/ts_util.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/ts_util.ts @@ -8,10 +8,7 @@ import ts from 'typescript'; -import {Import} from '../../translator'; - import {addExpressionIdentifier, ExpressionIdentifier} from './comments'; -import {wrapForTypeChecker} from './diagnostics'; @@ -163,11 +160,3 @@ export function tsNumericExpression(value: number): ts.NumericLiteral|ts.PrefixU return ts.factory.createNumericLiteral(value); } - -export function getImportString(imp: Import): string { - if (imp.qualifier === null) { - return `import from '${imp.specifier}';`; - } else { - return `import * as ${imp.qualifier.text} from '${imp.specifier}';`; - } -} diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts index 6f46600fb0b05..257100b91fd59 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts @@ -8,16 +8,15 @@ import ts from 'typescript'; import {AbsoluteFsPath, join} from '../../file_system'; -import {NoopImportRewriter, Reference, ReferenceEmitter} from '../../imports'; +import {Reference, ReferenceEmitter} from '../../imports'; import {ClassDeclaration, ReflectionHost} from '../../reflection'; -import {ImportManager} from '../../translator'; +import {ImportManagerV2} from '../../translator'; import {TypeCheckBlockMetadata, TypeCheckingConfig} from '../api'; import {DomSchemaChecker} from './dom'; import {Environment} from './environment'; import {OutOfBandDiagnosticRecorder} from './oob'; import {ensureTypeCheckFilePreparationImports} from './tcb_util'; -import {getImportString} from './ts_util'; import {generateTypeCheckBlock, TcbGenericContextBehavior} from './type_check_block'; @@ -38,7 +37,14 @@ export class TypeCheckFile extends Environment { readonly fileName: AbsoluteFsPath, config: TypeCheckingConfig, refEmitter: ReferenceEmitter, reflector: ReflectionHost, compilerHost: Pick) { super( - config, new ImportManager(new NoopImportRewriter(), 'i'), refEmitter, reflector, + config, new ImportManagerV2({ + // This minimizes noticeable changes with older versions of `ImportManager`. + forceGenerateNamespacesForNewImports: true, + // Type check block code affects code completion and fix suggestions. + // We want to encourage single quotes for now, like we always did. + shouldUseSingleQuotes: () => true + }), + refEmitter, reflector, ts.createSourceFile( compilerHost.getCanonicalFileName(fileName), '', ts.ScriptTarget.Latest, true)); } @@ -60,11 +66,21 @@ export class TypeCheckFile extends Environment { // import to e.g. `@angular/core` always exists to guarantee a stable graph. ensureTypeCheckFilePreparationImports(this); - let source: string = this.importManager.getAllImports(this.contextFile.fileName) - .map(getImportString) - .join('\n') + - '\n\n'; + const importChanges = this.importManager.finalize(); + if (importChanges.updatedImports.size > 0) { + throw new Error( + 'AssertionError: Expected no imports to be updated for a new type check file.'); + } + const printer = ts.createPrinter({removeComments}); + let source = ''; + + const newImports = importChanges.newImports.get(this.contextFile.fileName); + if (newImports !== undefined) { + source += newImports.map(i => printer.printNode(ts.EmitHint.Unspecified, i, this.contextFile)) + .join('\n'); + } + source += '\n'; for (const stmt of this.pipeInstStatements) { source += printer.printNode(ts.EmitHint.Unspecified, stmt, this.contextFile) + '\n'; diff --git a/yarn.lock b/yarn.lock index 75a944222f85c..10a3e9d640bd8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -386,7 +386,6 @@ "@angular/build-tooling@https://github.com/angular/dev-infra-private-build-tooling-builds.git#65d002a534a74daa3c9bd26bdea5a092cbd519df": version "0.0.0-5774b71c01a55c4c998f858ee37d3b77ae704c31" - uid "65d002a534a74daa3c9bd26bdea5a092cbd519df" resolved "https://github.com/angular/dev-infra-private-build-tooling-builds.git#65d002a534a74daa3c9bd26bdea5a092cbd519df" dependencies: "@angular-devkit/build-angular" "17.2.0-rc.0" @@ -495,7 +494,6 @@ "@angular/docs@https://github.com/angular/dev-infra-private-docs-builds.git#82c4573f5c9d4fb864271a1c74259fc251457e2f": version "0.0.0-5774b71c01a55c4c998f858ee37d3b77ae704c31" - uid "82c4573f5c9d4fb864271a1c74259fc251457e2f" resolved "https://github.com/angular/dev-infra-private-docs-builds.git#82c4573f5c9d4fb864271a1c74259fc251457e2f" dependencies: "@angular/cdk" "17.3.0-next.0" @@ -4876,8 +4874,7 @@ "@types/glob" "*" "@types/node" "*" -"@types/selenium-webdriver4@npm:@types/selenium-webdriver@4.1.21", "@types/selenium-webdriver@^4.1.21": - name "@types/selenium-webdriver4" +"@types/selenium-webdriver4@npm:@types/selenium-webdriver@4.1.21": version "4.1.21" resolved "https://registry.yarnpkg.com/@types/selenium-webdriver/-/selenium-webdriver-4.1.21.tgz#79fe31faf9953a4143c3e32944d98d5146bbe185" integrity sha512-QGURnImvxYlIQz5DVhvHdqpYNLBjhJ2Vm+cnQI2G9QZzkWlZm0LkLcvDcHp+qE6N2KBz4CeuvXgPO7W3XQ0Tyw== @@ -4894,6 +4891,14 @@ resolved "https://registry.yarnpkg.com/@types/selenium-webdriver/-/selenium-webdriver-3.0.25.tgz#7982bf5add9539b4e953512354055545e8508cff" integrity sha512-gkb8rx0kjHuaVUzIfKa1tVVjnEmjYLufDmoOHKPsaoRraTcNjV9W9ruFWi3Xv821c7M5gZVmrGOT8BJYsY/acQ== +"@types/selenium-webdriver@^4.1.21": + name "@types/selenium-webdriver4" + version "4.1.22" + resolved "https://registry.yarnpkg.com/@types/selenium-webdriver/-/selenium-webdriver-4.1.22.tgz#344519b90727eb713e1ce6d2e0198eb0b4f8f316" + integrity sha512-MCL4l7q8dwxejr2Q2NXLyNwHWMPdlWE0Kpn6fFwJtvkJF7PTkG5jkvbH/X1IAAQxgt/L1dA8u2GtDeekvSKvOA== + dependencies: + "@types/ws" "*" + "@types/semver@^7.3.4": version "7.5.4" resolved "https://registry.yarnpkg.com/@types/semver/-/semver-7.5.4.tgz#0a41252ad431c473158b22f9bfb9a63df7541cff" @@ -12480,7 +12485,7 @@ magic-string@0.30.7: dependencies: "@jridgewell/sourcemap-codec" "^1.4.15" -magic-string@0.30.8: +magic-string@0.30.8, magic-string@^0.30.8: version "0.30.8" resolved "https://registry.yarnpkg.com/magic-string/-/magic-string-0.30.8.tgz#14e8624246d2bedba70d5462aa99ac9681844613" integrity sha512-ISQTe55T2ao7XtlAStud6qwYPZjE4GK1S/BeVPus4jrq6JuOnQ00YKQC581RWhR122W7msZV263KzVeLoqidyQ==