From 543631f2b36dfb565b6b15abfdfddb0a3bfb30ef Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Mon, 26 Aug 2019 18:26:04 +0200 Subject: [PATCH] refactor(core): undecorated-classes migration should properly construct object literal from metadata (#32319) The `undecorated-classes-with-di` migration currently creates invalid object literals from parsed NGC metadata files if there are object literal properties with keys that contain special characters. e.g. consider a decorated base class with a host binding using `[class.X]`. Currently the migration parses and converts the metadata to TypeScript code but incorrectly uses `[class.X]` unquoted as identifier. PR Close #32319 --- .../convert_directive_metadata.ts | 21 ++++++++++++++++--- ...ecorated_classes_with_di_migration_spec.ts | 18 +++++++++++++--- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/packages/core/schematics/migrations/undecorated-classes-with-di/decorator_rewrite/convert_directive_metadata.ts b/packages/core/schematics/migrations/undecorated-classes-with-di/decorator_rewrite/convert_directive_metadata.ts index 7765cbfc05..aef0570b17 100644 --- a/packages/core/schematics/migrations/undecorated-classes-with-di/decorator_rewrite/convert_directive_metadata.ts +++ b/packages/core/schematics/migrations/undecorated-classes-with-di/decorator_rewrite/convert_directive_metadata.ts @@ -9,6 +9,9 @@ import {StaticSymbol} from '@angular/compiler'; import * as ts from 'typescript'; +/** Error that will be thrown if an unexpected value needs to be converted. */ +export class UnexpectedMetadataValueError extends Error {} + /** * Converts a directive metadata object into a TypeScript expression. Throws * if metadata cannot be cleanly converted. @@ -63,7 +66,7 @@ export function convertDirectiveMetadataToExpression( metadataValue, resolveSymbolImport, createImport, convertProperty); } - literalProperties.push(ts.createPropertyAssignment(key, propertyValue)); + literalProperties.push(ts.createPropertyAssignment(getPropertyName(key), propertyValue)); } return ts.createObjectLiteral(literalProperties, true); @@ -72,5 +75,17 @@ export function convertDirectiveMetadataToExpression( throw new UnexpectedMetadataValueError(); } -/** Error that will be thrown if a unexpected value needs to be converted. */ -export class UnexpectedMetadataValueError extends Error {} +/** + * Gets a valid property name from the given text. If the text cannot be used + * as unquoted identifier, the name will be wrapped in a string literal. +*/ +function getPropertyName(name: string): string|ts.StringLiteral { + // Matches the most common identifiers that do not need quotes. Constructing a + // regular expression that matches the ECMAScript specification in order to determine + // whether quotes are needed is out of scope for this migration. For those more complex + // property names, we just always use quotes (when constructing AST from metadata). + if (/^[a-zA-Z_$]+$/.test(name)) { + return name; + } + return ts.createStringLiteral(name); +} diff --git a/packages/core/schematics/test/undecorated_classes_with_di_migration_spec.ts b/packages/core/schematics/test/undecorated_classes_with_di_migration_spec.ts index 4c53708688..7f64e87558 100644 --- a/packages/core/schematics/test/undecorated_classes_with_di_migration_spec.ts +++ b/packages/core/schematics/test/undecorated_classes_with_di_migration_spec.ts @@ -1027,7 +1027,10 @@ describe('Undecorated classes with DI migration', () => { provide: NG_VALIDATORS, useExisting: BaseComponent, multi: true - }] + }], + host: { + "[class.is-enabled]": "isEnabled === true" + } }) export class PassThrough extends BaseComponent {}`); expect(tree.readContent('/index.ts')).toContain(dedent ` @@ -1040,7 +1043,10 @@ describe('Undecorated classes with DI migration', () => { provide: NG_VALIDATORS, useExisting: BaseComponent, multi: true - }] + }], + host: { + "[class.is-enabled]": "isEnabled === true" + } }) export class MyComp extends PassThrough {}`); expect(tree.readContent('/index.ts')).toContain(dedent ` @@ -1081,7 +1087,10 @@ describe('Undecorated classes with DI migration', () => { provide: NG_VALIDATORS, useExisting: BaseComponent, multi: true - }] + }], + host: { + "[class.is-enabled]": "isEnabled === true" + } }) export class MyComp extends BaseComponent {}`); }); @@ -1238,6 +1247,9 @@ describe('Undecorated classes with DI migration', () => { member: 'None' }, providers: [{__symbolic: 'reference', name: 'testValidators'}], + host: { + '[class.is-enabled]': 'isEnabled === true', + } }] }], members: {}