Skip to content

Commit

Permalink
refactor(migrations): support use of an ESM @angular/compiler-cli p…
Browse files Browse the repository at this point in the history 
…ackage (#43657)

Currently, migrations and schematics must be in CommonJS format. However, framework packages will only be ESM from v13 and onward. To support this configuration, dynamic import expressions are now used to load `@angular/compiler-cli` and its new secondary entry point `@angular/compiler-cli/private/migrations`. Dynamic imports within Node.js allow the `@angular/core` migrations’ CommonJS code to load ESM code. Unfortunately, TypeScript will currently, unconditionally down-level dynamic import into a require call. `require` calls cannot load ESM code and will result in a runtime error. To workaround this, a Function constructor is used to prevent TypeScript from changing the dynamic import. Once TypeScript provides support for keeping the dynamic import this workaround can be dropped and replaced with a standard dynamic import.  Due to the use of the dynamic import, a reference to the dynamically loaded modules must now be passed to all locations that use values from the modules.

PR Close #43657
  • Loading branch information
@clydin @dylhunn
clydin authored and dylhunn committed Oct 4, 2021
1 parent a268c44 commit e0015d3
Show file tree
Hide file tree
Showing 20 changed files with 303 additions and 135 deletions.
4 changes: 4 additions & 0 deletions packages/core/schematics/migrations/google3/BUILD.bazel
View file
Expand Up @@ -7,6 +7,10 @@ ts_library(
visibility = ["//packages/core/schematics/test/google3:__pkg__"],
deps = [
"//packages/compiler",
"//packages/compiler-cli/src/ngtsc/annotations",
"//packages/compiler-cli/src/ngtsc/imports",
"//packages/compiler-cli/src/ngtsc/partial_evaluator",
"//packages/compiler-cli/src/ngtsc/reflection",
"//packages/core/schematics/migrations/activated-route-snapshot-fragment",
"//packages/core/schematics/migrations/can-activate-with-redirect-to",
"//packages/core/schematics/migrations/dynamic-queries",
Expand Down
15 changes: 14 additions & 1 deletion packages/core/schematics/migrations/google3/noMissingInjectableRule.ts
View file
Expand Up @@ -6,6 +6,11 @@
* found in the LICENSE file at https://angular.io/license
*/

import {forwardRefResolver} from '@angular/compiler-cli/src/ngtsc/annotations';
import {Reference} from '@angular/compiler-cli/src/ngtsc/imports';
import {DynamicValue, PartialEvaluator} from '@angular/compiler-cli/src/ngtsc/partial_evaluator';
import {StaticInterpreter} from '@angular/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter';
import {reflectObjectLiteral, TypeScriptReflectionHost} from '@angular/compiler-cli/src/ngtsc/reflection';
import {RuleFailure, Rules} from 'tslint';
import ts from 'typescript';

Expand Down Expand Up @@ -34,7 +39,15 @@ export class Rule extends Rules.TypedRule {
sourceFiles.forEach(sourceFile => definitionCollector.visitNode(sourceFile));

const {resolvedModules, resolvedDirectives} = definitionCollector;
const transformer = new MissingInjectableTransform(typeChecker, getUpdateRecorder);
const transformer = new MissingInjectableTransform(typeChecker, getUpdateRecorder, {
Reference,
DynamicValue,
PartialEvaluator,
StaticInterpreter,
TypeScriptReflectionHost,
forwardRefResolver,
reflectObjectLiteral,
});
const updateRecorders = new Map<ts.SourceFile, TslintUpdateRecorder>();

[...transformer.migrateModules(resolvedModules),
Expand Down
16 changes: 14 additions & 2 deletions packages/core/schematics/migrations/google3/undecoratedClassesWithDecoratedFieldsRule.ts
View file
Expand Up @@ -5,7 +5,11 @@
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import {forwardRefResolver} from '@angular/compiler-cli/src/ngtsc/annotations';
import {Reference} from '@angular/compiler-cli/src/ngtsc/imports';
import {DynamicValue, PartialEvaluator} from '@angular/compiler-cli/src/ngtsc/partial_evaluator';
import {StaticInterpreter} from '@angular/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter';
import {reflectObjectLiteral, TypeScriptReflectionHost} from '@angular/compiler-cli/src/ngtsc/reflection';
import {RuleFailure, Rules} from 'tslint';
import ts from 'typescript';
import {TslintUpdateRecorder} from '../undecorated-classes-with-decorated-fields/google3/tslint_update_recorder';
Expand All @@ -23,7 +27,15 @@ export class Rule extends Rules.TypedRule {
s => !s.isDeclarationFile && !program.isSourceFileFromExternalLibrary(s));
const updateRecorders = new Map<ts.SourceFile, TslintUpdateRecorder>();
const transform =
new UndecoratedClassesWithDecoratedFieldsTransform(typeChecker, getUpdateRecorder);
new UndecoratedClassesWithDecoratedFieldsTransform(typeChecker, getUpdateRecorder, {
Reference,
DynamicValue,
PartialEvaluator,
StaticInterpreter,
TypeScriptReflectionHost,
forwardRefResolver,
reflectObjectLiteral,
});

// Migrate all source files in the project.
transform.migrate(sourceFiles);
Expand Down
23 changes: 20 additions & 3 deletions packages/core/schematics/migrations/missing-injectable/index.ts
View file
Expand Up @@ -9,6 +9,8 @@
import {Rule, SchematicContext, SchematicsException, Tree} from '@angular-devkit/schematics';
import {relative} from 'path';
import ts from 'typescript';

import {loadCompilerCliMigrationsModule, loadEsmModule} from '../../utils/load_esm';
import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {canMigrateFile, createMigrationProgram} from '../../utils/typescript/compiler_host';
import {NgDefinitionCollector} from './definition_collector';
Expand All @@ -28,8 +30,20 @@ export default function(): Rule {
'which don\'t have that decorator set.');
}

let compilerCliMigrationsModule;
try {
// Load ESM `@angular/compiler/private/migrations` using the TypeScript dynamic import
// workaround. Once TypeScript provides support for keeping the dynamic import this workaround
// can be changed to a direct dynamic import.
compilerCliMigrationsModule = await loadCompilerCliMigrationsModule();
} catch (e) {
throw new SchematicsException(
`Unable to load the '@angular/compiler-cli' package. Details: ${e.message}`);
}

for (const tsconfigPath of [...buildPaths, ...testPaths]) {
failures.push(...runMissingInjectableMigration(tree, tsconfigPath, basePath));
failures.push(...runMissingInjectableMigration(
tree, tsconfigPath, basePath, compilerCliMigrationsModule));
}

if (failures.length) {
Expand All @@ -41,7 +55,9 @@ export default function(): Rule {
}

function runMissingInjectableMigration(
tree: Tree, tsconfigPath: string, basePath: string): string[] {
tree: Tree, tsconfigPath: string, basePath: string,
compilerCliMigrationsModule: typeof import('@angular/compiler-cli/private/migrations')):
string[] {
const {program} = createMigrationProgram(tree, tsconfigPath, basePath);
const failures: string[] = [];
const typeChecker = program.getTypeChecker();
Expand All @@ -53,7 +69,8 @@ function runMissingInjectableMigration(
sourceFiles.forEach(sourceFile => definitionCollector.visitNode(sourceFile));

const {resolvedModules, resolvedDirectives} = definitionCollector;
const transformer = new MissingInjectableTransform(typeChecker, getUpdateRecorder);
const transformer =
new MissingInjectableTransform(typeChecker, getUpdateRecorder, compilerCliMigrationsModule);
const updateRecorders = new Map<ts.SourceFile, UpdateRecorder>();

[...transformer.migrateModules(resolvedModules),
Expand Down
86 changes: 52 additions & 34 deletions packages/core/schematics/migrations/missing-injectable/providers_evaluator.ts
View file
Expand Up @@ -6,51 +6,69 @@
* found in the LICENSE file at https://angular.io/license
*/

import {forwardRefResolver, ResolvedValue, StaticInterpreter} from '@angular/compiler-cli/private/migrations';
import ts from 'typescript';

import type {ResolvedValue, TypeScriptReflectionHost} from '@angular/compiler-cli/private/migrations';

export interface ProviderLiteral {
node: ts.ObjectLiteralExpression;
resolvedValue: ResolvedValue;
}

/**
* Providers evaluator that extends the ngtsc static interpreter. This is necessary because
* the static interpreter by default only exposes the resolved value, but we are also interested
* in the TypeScript nodes that declare providers. It would be possible to manually traverse the
* AST to collect these nodes, but that would mean that we need to re-implement the static
* interpreter in order to handle all possible scenarios. (e.g. spread operator, function calls,
* callee scope). This can be avoided by simply extending the static interpreter and intercepting
* the "visitObjectLiteralExpression" method.
* A factory function to create an evaluator for providers. This is required to be a
* factory function because the underlying class extends a class that is only available
* from within a dynamically imported module (`@angular/compiler-cli/private/migrations`)
* and is therefore not available at module evaluation time.
*/
export class ProvidersEvaluator extends StaticInterpreter {
private _providerLiterals: ProviderLiteral[] = [];
export function createProvidersEvaluator(
compilerCliMigrationsModule: typeof import('@angular/compiler-cli/private/migrations'),
host: TypeScriptReflectionHost, checker: ts.TypeChecker): {
evaluate:
(expr: ts.Expression) => {
resolvedValue: ResolvedValue, literals: ProviderLiteral[]
}
} {
/**
* Providers evaluator that extends the ngtsc static interpreter. This is necessary because
* the static interpreter by default only exposes the resolved value, but we are also interested
* in the TypeScript nodes that declare providers. It would be possible to manually traverse the
* AST to collect these nodes, but that would mean that we need to re-implement the static
* interpreter in order to handle all possible scenarios. (e.g. spread operator, function calls,
* callee scope). This can be avoided by simply extending the static interpreter and intercepting
* the "visitObjectLiteralExpression" method.
*/
class ProvidersEvaluator extends compilerCliMigrationsModule.StaticInterpreter {
private _providerLiterals: ProviderLiteral[] = [];

override visitObjectLiteralExpression(node: ts.ObjectLiteralExpression, context: any) {
const resolvedValue =
super.visitObjectLiteralExpression(node, {...context, insideProviderDef: true});
// do not collect nested object literals. e.g. a provider could use a
// spread assignment (which resolves to another object literal). In that
// case the referenced object literal is not a provider object literal.
if (!context.insideProviderDef) {
this._providerLiterals.push({node, resolvedValue});
override visitObjectLiteralExpression(node: ts.ObjectLiteralExpression, context: any) {
const resolvedValue =
super.visitObjectLiteralExpression(node, {...context, insideProviderDef: true});
// do not collect nested object literals. e.g. a provider could use a
// spread assignment (which resolves to another object literal). In that
// case the referenced object literal is not a provider object literal.
if (!context.insideProviderDef) {
this._providerLiterals.push({node, resolvedValue});
}
return resolvedValue;
}
return resolvedValue;
}

/**
* Evaluates the given expression and returns its statically resolved value
* and a list of object literals which define Angular providers.
*/
evaluate(expr: ts.Expression) {
this._providerLiterals = [];
const resolvedValue = this.visit(expr, {
originatingFile: expr.getSourceFile(),
absoluteModuleName: null,
resolutionContext: expr.getSourceFile().fileName,
scope: new Map(),
foreignFunctionResolver: forwardRefResolver
});
return {resolvedValue, literals: this._providerLiterals};
/**
* Evaluates the given expression and returns its statically resolved value
* and a list of object literals which define Angular providers.
*/
evaluate(expr: ts.Expression) {
this._providerLiterals = [];
const resolvedValue = this.visit(expr, {
originatingFile: expr.getSourceFile(),
absoluteModuleName: null,
resolutionContext: expr.getSourceFile().fileName,
scope: new Map(),
foreignFunctionResolver: compilerCliMigrationsModule.forwardRefResolver
});
return {resolvedValue, literals: this._providerLiterals};
}
}

return new ProvidersEvaluator(host, checker, /* dependencyTracker */ null);
}
20 changes: 12 additions & 8 deletions packages/core/schematics/migrations/missing-injectable/transform.ts
View file
Expand Up @@ -6,14 +6,14 @@
* found in the LICENSE file at https://angular.io/license
*/

import {DynamicValue, Reference, ResolvedValue, TypeScriptReflectionHost} from '@angular/compiler-cli/private/migrations';
import type {ResolvedValue} from '@angular/compiler-cli/private/migrations';
import ts from 'typescript';

import {ImportManager} from '../../utils/import_manager';
import {getAngularDecorators} from '../../utils/ng_decorators';

import {ResolvedDirective, ResolvedNgModule} from './definition_collector';
import {ProviderLiteral, ProvidersEvaluator} from './providers_evaluator';
import {createProvidersEvaluator, ProviderLiteral} from './providers_evaluator';
import {UpdateRecorder} from './update_recorder';

/**
Expand All @@ -33,7 +33,7 @@ export interface AnalysisFailure {
export class MissingInjectableTransform {
private printer = ts.createPrinter();
private importManager = new ImportManager(this.getUpdateRecorder, this.printer);
private providersEvaluator: ProvidersEvaluator;
private providersEvaluator;

/** Set of provider class declarations which were already checked or migrated. */
private visitedProviderClasses = new Set<ts.ClassDeclaration>();
Expand All @@ -43,9 +43,12 @@ export class MissingInjectableTransform {

constructor(
private typeChecker: ts.TypeChecker,
private getUpdateRecorder: (sf: ts.SourceFile) => UpdateRecorder) {
this.providersEvaluator = new ProvidersEvaluator(
new TypeScriptReflectionHost(typeChecker), typeChecker, /* dependencyTracker */ null);
private getUpdateRecorder: (sf: ts.SourceFile) => UpdateRecorder,
private compilerCliMigrationsModule:
typeof import('@angular/compiler-cli/private/migrations')) {
this.providersEvaluator = createProvidersEvaluator(
compilerCliMigrationsModule,
new compilerCliMigrationsModule.TypeScriptReflectionHost(typeChecker), typeChecker);
}

recordChanges() {
Expand Down Expand Up @@ -214,7 +217,8 @@ export class MissingInjectableTransform {
*/
private _visitProviderResolvedValue(value: ResolvedValue, module: ResolvedNgModule):
AnalysisFailure[] {
if (value instanceof Reference && ts.isClassDeclaration(value.node)) {
if (value instanceof this.compilerCliMigrationsModule.Reference &&
ts.isClassDeclaration(value.node)) {
this.migrateProviderClass(value.node, module);
} else if (value instanceof Map) {
// If a "ClassProvider" has the "deps" property set, then we do not need to
Expand All @@ -227,7 +231,7 @@ export class MissingInjectableTransform {
return value.reduce(
(res, v) => res.concat(this._visitProviderResolvedValue(v, module)),
[] as AnalysisFailure[]);
} else if (value instanceof DynamicValue) {
} else if (value instanceof this.compilerCliMigrationsModule.DynamicValue) {
return [{node: value.node, message: `Provider is not statically analyzable.`}];
}
return [];
Expand Down
22 changes: 19 additions & 3 deletions packages/core/schematics/migrations/module-with-providers/index.ts
View file
Expand Up @@ -10,6 +10,7 @@ import {Rule, SchematicContext, SchematicsException, Tree, UpdateRecorder} from
import {relative} from 'path';
import ts from 'typescript';

import {loadCompilerCliMigrationsModule, loadEsmModule} from '../../utils/load_esm';
import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {canMigrateFile, createMigrationProgram} from '../../utils/typescript/compiler_host';

Expand All @@ -32,8 +33,20 @@ export default function(): Rule {
'Could not find any tsconfig file. Cannot migrate ModuleWithProviders.');
}

let compilerCliMigrationsModule;
try {
// Load ESM `@angular/compiler/private/migrations` using the TypeScript dynamic import
// workaround. Once TypeScript provides support for keeping the dynamic import this workaround
// can be changed to a direct dynamic import.
compilerCliMigrationsModule = await loadCompilerCliMigrationsModule();
} catch (e) {
throw new SchematicsException(
`Unable to load the '@angular/compiler-cli' package. Details: ${e.message}`);
}

for (const tsconfigPath of allPaths) {
failures.push(...runModuleWithProvidersMigration(tree, tsconfigPath, basePath));
failures.push(...runModuleWithProvidersMigration(
tree, tsconfigPath, basePath, compilerCliMigrationsModule));
}

if (failures.length) {
Expand All @@ -44,7 +57,9 @@ export default function(): Rule {
};
}

function runModuleWithProvidersMigration(tree: Tree, tsconfigPath: string, basePath: string) {
function runModuleWithProvidersMigration(
tree: Tree, tsconfigPath: string, basePath: string,
compilerCliMigrationsModule: typeof import('@angular/compiler-cli/private/migrations')) {
const {program} = createMigrationProgram(tree, tsconfigPath, basePath);
const failures: string[] = [];
const typeChecker = program.getTypeChecker();
Expand All @@ -56,7 +71,8 @@ function runModuleWithProvidersMigration(tree: Tree, tsconfigPath: string, baseP
sourceFiles.forEach(sourceFile => collector.visitNode(sourceFile));

const {resolvedModules, resolvedNonGenerics} = collector;
const transformer = new ModuleWithProvidersTransform(typeChecker, getUpdateRecorder);
const transformer =
new ModuleWithProvidersTransform(typeChecker, getUpdateRecorder, compilerCliMigrationsModule);
const updateRecorders = new Map<ts.SourceFile, UpdateRecorder>();

[...resolvedModules.reduce(
Expand Down
17 changes: 10 additions & 7 deletions packages/core/schematics/migrations/module-with-providers/transform.ts
View file
Expand Up @@ -7,7 +7,7 @@
*/

import {UpdateRecorder} from '@angular-devkit/schematics';
import {DynamicValue, PartialEvaluator, Reference, ResolvedValue, ResolvedValueMap, TypeScriptReflectionHost} from '@angular/compiler-cli/private/migrations';
import type {ResolvedValue, ResolvedValueMap} from '@angular/compiler-cli/private/migrations';
import ts from 'typescript';

import {ResolvedNgModule} from './collector';
Expand All @@ -22,13 +22,16 @@ const TODO_COMMENT = 'TODO: The following node requires a generic type for `Modu

export class ModuleWithProvidersTransform {
private printer = ts.createPrinter();
private partialEvaluator: PartialEvaluator = new PartialEvaluator(
new TypeScriptReflectionHost(this.typeChecker), this.typeChecker,
private partialEvaluator = new this.compilerCliMigrationsModule.PartialEvaluator(
new this.compilerCliMigrationsModule.TypeScriptReflectionHost(this.typeChecker),
this.typeChecker,
/* dependencyTracker */ null);

constructor(
private typeChecker: ts.TypeChecker,
private getUpdateRecorder: (sf: ts.SourceFile) => UpdateRecorder) {}
private getUpdateRecorder: (sf: ts.SourceFile) => UpdateRecorder,
private compilerCliMigrationsModule:
typeof import('@angular/compiler-cli/private/migrations')) {}

/** Migrates a given NgModule by walking through the referenced providers and static methods. */
migrateModule(module: ResolvedNgModule): AnalysisFailure[] {
Expand Down Expand Up @@ -132,10 +135,10 @@ export class ModuleWithProvidersTransform {
private _getTypeOfResolvedValue(value: ResolvedValue): string|undefined {
if (value instanceof Map && this.isModuleWithProvidersType(value)) {
const mapValue = value.get('ngModule')!;
if (mapValue instanceof Reference && ts.isClassDeclaration(mapValue.node) &&
mapValue.node.name) {
if (mapValue instanceof this.compilerCliMigrationsModule.Reference &&
ts.isClassDeclaration(mapValue.node) && mapValue.node.name) {
return mapValue.node.name.text;
} else if (mapValue instanceof DynamicValue) {
} else if (mapValue instanceof this.compilerCliMigrationsModule.DynamicValue) {
addTodoToNode(mapValue.node, TODO_COMMENT);
this._updateNode(mapValue.node, mapValue.node);
}
Expand Down

0 comments on commit e0015d3

Please sign in to comment.