Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor(transformers): update downlevel ctor transformer #730

Merged
merged 1 commit into from
Jan 14, 2021
Merged

refactor(transformers): update downlevel ctor transformer #730

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
139 changes: 12 additions & 127 deletions src/transformers/downlevel-ctor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,107 +11,7 @@
import { Decorator, ReflectionHost, TypeScriptReflectionHost } from '@angular/compiler-cli/src/ngtsc/reflection';
import ts from 'typescript';

/**
* Describes a TypeScript transformation context with the internal emit
* resolver exposed. There are requests upstream in TypeScript to expose
* that as public API: https://github.com/microsoft/TypeScript/issues/17516..
*/
interface TransformationContextWithResolver extends ts.TransformationContext {
getEmitResolver: () => EmitResolver;
}

/** Describes a subset of the TypeScript internal emit resolver. */
interface EmitResolver {
isReferencedAliasDeclaration?(node: ts.Node, checkChildren?: boolean): void;
}

/**
* Patches the alias declaration reference resolution for a given transformation context
* so that TypeScript knows about the specified alias declarations being referenced.
*
* This exists because TypeScript performs analysis of import usage before transformers
* run and doesn't refresh its state after transformations. This means that imports
* for symbols used as constructor types are elided due to their original type-only usage.
*
* In reality though, since we downlevel decorators and constructor parameters, we want
* these symbols to be retained in the JavaScript output as they will be used as values
* at runtime. We can instruct TypeScript to preserve imports for such identifiers by
* creating a mutable clone of a given import specifier/clause or namespace, but that
* has the downside of preserving the full import in the JS output. See:
* https://github.com/microsoft/TypeScript/blob/3eaa7c65f6f076a08a5f7f1946fd0df7c7430259/src/compiler/transformers/ts.ts#L242-L250.
*
* This is a trick the CLI used in the past for constructor parameter downleveling in JIT:
* https://github.com/angular/angular-cli/blob/b3f84cc5184337666ce61c07b7b9df418030106f/packages/ngtools/webpack/src/transformers/ctor-parameters.ts#L323-L325
* The trick is not ideal though as it preserves the full import (as outlined before), and it
* results in a slow-down due to the type checker being involved multiple times. The CLI
* worked around this import preserving issue by having another complex post-process step that
* detects and elides unused imports. Note that these unused imports could cause unused chunks
* being generated by Webpack if the application or library is not marked as side-effect free.
*
* This is not ideal though, as we basically re-implement the complex import usage resolution
* from TypeScript. We can do better by letting TypeScript do the import eliding, but providing
* information about the alias declarations (e.g. import specifiers) that should not be elided
* because they are actually referenced (as they will now appear in static properties).
*
* More information about these limitations with transformers can be found in:
* 1. https://github.com/Microsoft/TypeScript/issues/17552.
* 2. https://github.com/microsoft/TypeScript/issues/17516.
* 3. https://github.com/angular/tsickle/issues/635.
*
* The patch we apply to tell TypeScript about actual referenced aliases (i.e. imported symbols),
* matches conceptually with the logic that runs internally in TypeScript when the
* `emitDecoratorMetadata` flag is enabled. TypeScript basically surfaces the same problem and
* solves it conceptually the same way, but obviously doesn't need to access an `@internal` API.
*
* See below. Note that this uses sourcegraph as the TypeScript checker file doesn't display on
* Github.
* https://sourcegraph.com/github.com/microsoft/TypeScript@3eaa7c65f6f076a08a5f7f1946fd0df7c7430259/-/blob/src/compiler/checker.ts#L31219-31257
*/
function patchAliasReferenceResolutionOrDie(
context: ts.TransformationContext,
referencedAliases: Set<ts.Declaration>,
): void {
// If the `getEmitResolver` method is not available, TS most likely changed the
// internal structure of the transformation context. We will abort gracefully.
if (!isTransformationContextWithEmitResolver(context)) {
throwIncompatibleTransformationContextError();

return;
}
const emitResolver = context.getEmitResolver();
// eslint-disable-next-line @typescript-eslint/unbound-method
const originalReferenceResolution = emitResolver.isReferencedAliasDeclaration;
// If the emit resolver does not have a function called `isReferencedAliasDeclaration`, then
// we abort gracefully as most likely TS changed the internal structure of the emit resolver.
if (originalReferenceResolution === undefined) {
throwIncompatibleTransformationContextError();

return;
}
emitResolver.isReferencedAliasDeclaration = function (node, ...args) {
if (isAliasImportDeclaration(node) && referencedAliases.has(node)) {
return true;
}

return originalReferenceResolution.call(emitResolver, node, ...args);
};
}

/** Whether the transformation context exposes its emit resolver. */
function isTransformationContextWithEmitResolver(
context: ts.TransformationContext,
): context is TransformationContextWithResolver {
return (context as Partial<TransformationContextWithResolver>).getEmitResolver !== undefined;
}

/**
* Gets whether a given node corresponds to an import alias declaration. Alias
* declarations can be import specifiers, namespace imports or import clauses
* as these do not declare an actual symbol but just point to a target declaration.
*/
function isAliasImportDeclaration(node: ts.Node): node is ts.ImportSpecifier | ts.NamespaceImport | ts.ImportClause {
return ts.isImportSpecifier(node) || ts.isNamespaceImport(node) || ts.isImportClause(node);
}
import { isAliasImportDeclaration, loadIsReferencedAliasDeclarationPatch } from './patch-alias-reference-resolution';

/**
* Whether a given decorator should be treated as an Angular decorator.
Expand Down Expand Up @@ -532,7 +432,12 @@ function getDownlevelDecoratorsTransform(
skipClassDecorators: boolean,
): ts.TransformerFactory<ts.SourceFile> {
return (context: ts.TransformationContext) => {
const referencedParameterTypes = new Set<ts.Declaration>();
// Ensure that referenced type symbols are not elided by TypeScript. Imports for
// such parameter type symbols previously could be type-only, but now might be also
// used in the `ctorParameters` static property as a value. We want to make sure
// that TypeScript does not elide imports for such type references. Read more
// about this in the description for `loadIsReferencedAliasDeclarationPatch`.
const referencedParameterTypes = loadIsReferencedAliasDeclarationPatch(context);

/**
* Converts an EntityName (from a type annotation) to an expression (accessing a value).
Expand Down Expand Up @@ -627,7 +532,7 @@ function getDownlevelDecoratorsTransform(
return [undefined, element, []];
}

const name = element.name.text;
const name = (element.name as ts.Identifier).text;
const mutable = ts.getMutableClone(element);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(mutable as any).decorators = decoratorsToKeep.length
Expand Down Expand Up @@ -663,14 +568,16 @@ function getDownlevelDecoratorsTransform(
decoratorsToKeep.push(decoratorNode);
continue;
}
paramInfo.decorators.push(decoratorNode);
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
paramInfo!.decorators.push(decoratorNode);
}
if (param.type) {
// param has a type provided, e.g. "foo: Bar".
// The type will be emitted as a value expression in entityNameToExpression, which takes
// care not to emit anything for types that cannot be expressed as a value (e.g.
// interfaces).
paramInfo.type = param.type;
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
paramInfo!.type = param.type;
}
parametersInfo.push(paramInfo);
const newParam = ts.updateParameter(
Expand Down Expand Up @@ -815,13 +722,6 @@ function getDownlevelDecoratorsTransform(
}

return (sf: ts.SourceFile) => {
// Ensure that referenced type symbols are not elided by TypeScript. Imports for
// such parameter type symbols previously could be type-only, but now might be also
// used in the `ctorParameters` static property as a value. We want to make sure
// that TypeScript does not elide imports for such type references. Read more
// about this in the description for `patchAliasReferenceResolution`.
patchAliasReferenceResolutionOrDie(context, referencedParameterTypes);

// Downlevel decorators and constructor parameter types. We will keep track of all
// referenced constructor parameter types so that we can instruct TypeScript to
// not elide their imports if they previously were only type-only.
Expand All @@ -830,21 +730,6 @@ function getDownlevelDecoratorsTransform(
};
}

/**
* Throws an error about an incompatible TypeScript version for which the alias
* declaration reference resolution could not be monkey-patched. The error will
* also propose potential solutions that can be applied by developers.
*/
function throwIncompatibleTransformationContextError() {
throw Error(
'Unable to downlevel Angular decorators due to an incompatible TypeScript ' +
'version.\nIf you recently updated TypeScript and this issue surfaces now, consider ' +
'downgrading.\n\n' +
'Please report an issue on the Angular repositories when this issue ' +
'surfaces and you are using a supposedly compatible TypeScript version.',
);
}

/**
* Transform for downleveling Angular decorators and Angular-decorated class constructor
* parameters for dependency injection. This transform can be used by the CLI for JIT-mode
Expand Down
140 changes: 140 additions & 0 deletions src/transformers/patch-alias-reference-resolution.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* 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 ts from 'typescript';

/**
* Describes a TypeScript transformation context with the internal emit
* resolver exposed. There are requests upstream in TypeScript to expose
* that as public API: https://github.com/microsoft/TypeScript/issues/17516..
*/
interface TransformationContextWithResolver extends ts.TransformationContext {
getEmitResolver: () => EmitResolver;
}

const patchedReferencedAliasesSymbol = Symbol('patchedReferencedAliases');

/** Describes a subset of the TypeScript internal emit resolver. */
interface EmitResolver {
isReferencedAliasDeclaration?(node: ts.Node, ...args: unknown[]): void;
[patchedReferencedAliasesSymbol]?: Set<ts.Declaration>;
}

/**
* Patches the alias declaration reference resolution for a given transformation context
* so that TypeScript knows about the specified alias declarations being referenced.
*
* This exists because TypeScript performs analysis of import usage before transformers
* run and doesn't refresh its state after transformations. This means that imports
* for symbols used as constructor types are elided due to their original type-only usage.
*
* In reality though, since we downlevel decorators and constructor parameters, we want
* these symbols to be retained in the JavaScript output as they will be used as values
* at runtime. We can instruct TypeScript to preserve imports for such identifiers by
* creating a mutable clone of a given import specifier/clause or namespace, but that
* has the downside of preserving the full import in the JS output. See:
* https://github.com/microsoft/TypeScript/blob/3eaa7c65f6f076a08a5f7f1946fd0df7c7430259/src/compiler/transformers/ts.ts#L242-L250.
*
* This is a trick the CLI used in the past for constructor parameter downleveling in JIT:
* https://github.com/angular/angular-cli/blob/b3f84cc5184337666ce61c07b7b9df418030106f/packages/ngtools/webpack/src/transformers/ctor-parameters.ts#L323-L325
* The trick is not ideal though as it preserves the full import (as outlined before), and it
* results in a slow-down due to the type checker being involved multiple times. The CLI worked
* around this import preserving issue by having another complex post-process step that detects and
* elides unused imports. Note that these unused imports could cause unused chunks being generated
* by Webpack if the application or library is not marked as side-effect free.
*
* This is not ideal though, as we basically re-implement the complex import usage resolution
* from TypeScript. We can do better by letting TypeScript do the import eliding, but providing
* information about the alias declarations (e.g. import specifiers) that should not be elided
* because they are actually referenced (as they will now appear in static properties).
*
* More information about these limitations with transformers can be found in:
* 1. https://github.com/Microsoft/TypeScript/issues/17552.
* 2. https://github.com/microsoft/TypeScript/issues/17516.
* 3. https://github.com/angular/tsickle/issues/635.
*
* The patch we apply to tell TypeScript about actual referenced aliases (i.e. imported symbols),
* matches conceptually with the logic that runs internally in TypeScript when the
* `emitDecoratorMetadata` flag is enabled. TypeScript basically surfaces the same problem and
* solves it conceptually the same way, but obviously doesn't need to access an `@internal` API.
*
* The set that is returned by this function is meant to be filled with import declaration nodes
* that have been referenced in a value-position by the transform, such the the installed patch can
* ensure that those import declarations are not elided.
*
* See below. Note that this uses sourcegraph as the TypeScript checker file doesn't display on
* Github.
* https://sourcegraph.com/github.com/microsoft/TypeScript@3eaa7c65f6f076a08a5f7f1946fd0df7c7430259/-/blob/src/compiler/checker.ts#L31219-31257
*/
export function loadIsReferencedAliasDeclarationPatch(context: ts.TransformationContext): Set<ts.Declaration> {
// If the `getEmitResolver` method is not available, TS most likely changed the
// internal structure of the transformation context. We will abort gracefully.
if (!isTransformationContextWithEmitResolver(context)) {
throwIncompatibleTransformationContextError();
}
const emitResolver = context.getEmitResolver();

// The emit resolver may have been patched already, in which case we return the set of referenced
// aliases that was created when the patch was first applied.
// See https://github.com/angular/angular/issues/40276.
const existingReferencedAliases = emitResolver[patchedReferencedAliasesSymbol];
if (existingReferencedAliases !== undefined) {
return existingReferencedAliases;
}

const originalIsReferencedAliasDeclaration = emitResolver.isReferencedAliasDeclaration;
// If the emit resolver does not have a function called `isReferencedAliasDeclaration`, then
// we abort gracefully as most likely TS changed the internal structure of the emit resolver.
if (originalIsReferencedAliasDeclaration === undefined) {
throwIncompatibleTransformationContextError();
}

const referencedAliases = new Set<ts.Declaration>();
emitResolver.isReferencedAliasDeclaration = function (node, ...args) {
if (isAliasImportDeclaration(node) && referencedAliases.has(node)) {
return true;
}

return originalIsReferencedAliasDeclaration.call(emitResolver, node, ...args);
};

return (emitResolver[patchedReferencedAliasesSymbol] = referencedAliases);
}

/**
* Gets whether a given node corresponds to an import alias declaration. Alias
* declarations can be import specifiers, namespace imports or import clauses
* as these do not declare an actual symbol but just point to a target declaration.
*/
export function isAliasImportDeclaration(
node: ts.Node,
): node is ts.ImportSpecifier | ts.NamespaceImport | ts.ImportClause {
return ts.isImportSpecifier(node) || ts.isNamespaceImport(node) || ts.isImportClause(node);
}

/** Whether the transformation context exposes its emit resolver. */
function isTransformationContextWithEmitResolver(
context: ts.TransformationContext,
): context is TransformationContextWithResolver {
return (context as Partial<TransformationContextWithResolver>).getEmitResolver !== undefined;
}

/**
* Throws an error about an incompatible TypeScript version for which the alias
* declaration reference resolution could not be monkey-patched. The error will
* also propose potential solutions that can be applied by developers.
*/
function throwIncompatibleTransformationContextError(): never {
throw Error(
'Unable to downlevel Angular decorators due to an incompatible TypeScript ' +
'version.\nIf you recently updated TypeScript and this issue surfaces now, consider ' +
'downgrading.\n\n' +
'Please report an issue on the Angular repositories when this issue ' +
'surfaces and you are using a supposedly compatible TypeScript version.',
);
}