From b77b95071dbc9f9aa22067123a4fa8387832b36a Mon Sep 17 00:00:00 2001 From: Payam Valadkhan Date: Tue, 19 Dec 2023 17:14:15 -0500 Subject: [PATCH] fix(compiler-cli): input transform in local compilation mode (#53645) Currently compiling input transform in local mode breaks, since compiler does static analysis for the transform function, and this cannot be done in local mode if the function is imported from another compilation unit. In this fix the static analysis is ditched in local mode. PR Close #53645 --- .../ngtsc/annotations/directive/src/shared.ts | 47 ++++++-- .../test/ngtsc/local_compilation_spec.ts | 104 ++++++++++++++++++ 2 files changed, 139 insertions(+), 12 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts b/packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts index 8c0d839b6658b..adec2145d80de 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts @@ -78,9 +78,10 @@ export function extractDirectiveMetadata( // Construct the map of inputs both from the @Directive/@Component // decorator, and the decorated fields. - const inputsFromMeta = parseInputsArray(clazz, directive, evaluator, reflector, refEmitter); - const inputsFromFields = - parseInputFields(clazz, members, evaluator, reflector, refEmitter, coreModule); + const inputsFromMeta = + parseInputsArray(clazz, directive, evaluator, reflector, refEmitter, compilationMode); + const inputsFromFields = parseInputFields( + clazz, members, evaluator, reflector, refEmitter, coreModule, compilationMode); const inputs = ClassPropertyMapping.fromMappedObject({...inputsFromMeta, ...inputsFromFields}); // And outputs. @@ -616,8 +617,8 @@ function parseDecoratedFields( /** Parses the `inputs` array of a directive/component decorator. */ function parseInputsArray( clazz: ClassDeclaration, decoratorMetadata: Map, - evaluator: PartialEvaluator, reflector: ReflectionHost, - refEmitter: ReferenceEmitter): Record { + evaluator: PartialEvaluator, reflector: ReflectionHost, refEmitter: ReferenceEmitter, + compilationMode: CompilationMode): Record { const inputsField = decoratorMetadata.get('inputs'); if (inputsField === undefined) { @@ -669,7 +670,7 @@ function parseInputsArray( } transform = parseDecoratorInputTransformFunction( - clazz, name, transformValue, reflector, refEmitter); + clazz, name, transformValue, reflector, refEmitter, compilationMode); } inputs[name] = { @@ -711,8 +712,8 @@ function tryGetDecoratorOnMember( function tryParseInputFieldMapping( clazz: ClassDeclaration, member: ClassMember, evaluator: PartialEvaluator, - reflector: ReflectionHost, coreModule: string|undefined, - refEmitter: ReferenceEmitter): InputMapping|null { + reflector: ReflectionHost, coreModule: string|undefined, refEmitter: ReferenceEmitter, + compilationMode: CompilationMode): InputMapping|null { const classPropertyName = member.name; // Look for a decorator first. @@ -757,7 +758,7 @@ function tryParseInputFieldMapping( } transform = parseDecoratorInputTransformFunction( - clazz, classPropertyName, transformValue, reflector, refEmitter); + clazz, classPropertyName, transformValue, reflector, refEmitter, compilationMode); } return { @@ -797,8 +798,8 @@ function tryParseInputFieldMapping( /** Parses the class members that declare inputs (via decorator or initializer). */ function parseInputFields( clazz: ClassDeclaration, members: ClassMember[], evaluator: PartialEvaluator, - reflector: ReflectionHost, refEmitter: ReferenceEmitter, - coreModule: string|undefined): Record { + reflector: ReflectionHost, refEmitter: ReferenceEmitter, coreModule: string|undefined, + compilationMode: CompilationMode): Record { const inputs = {} as Record; for (const member of members) { @@ -814,6 +815,7 @@ function parseInputFields( reflector, coreModule, refEmitter, + compilationMode, ); if (inputMapping !== null) { inputs[classPropertyName] = inputMapping; @@ -836,7 +838,28 @@ function parseInputFields( */ export function parseDecoratorInputTransformFunction( clazz: ClassDeclaration, classPropertyName: string, value: DynamicValue|Reference, - reflector: ReflectionHost, refEmitter: ReferenceEmitter): DecoratorInputTransform { + reflector: ReflectionHost, refEmitter: ReferenceEmitter, + compilationMode: CompilationMode): DecoratorInputTransform { + // In local compilation mode we can skip type checking the function args. This is because usually + // the type check is done in a separate build which runs in full compilation mode. So here we skip + // all the diagnostics. + if (compilationMode === CompilationMode.LOCAL) { + const node = + value instanceof Reference ? value.getIdentityIn(clazz.getSourceFile()) : value.node; + + // This should never be null since we know the reference originates + // from the same file, but we null check it just in case. + if (node === null) { + throw createValueHasWrongTypeError( + value.node, value, 'Input transform function could not be referenced'); + } + + return { + node, + type: new Reference(ts.factory.createKeywordTypeNode(ts.SyntaxKind.UnknownKeyword)) + }; + } + const definition = reflector.getDefinitionOfFunction(value.node); if (definition === null) { diff --git a/packages/compiler-cli/test/ngtsc/local_compilation_spec.ts b/packages/compiler-cli/test/ngtsc/local_compilation_spec.ts index 4d9cf5fb33dc4..c25bdfcbd4b4e 100644 --- a/packages/compiler-cli/test/ngtsc/local_compilation_spec.ts +++ b/packages/compiler-cli/test/ngtsc/local_compilation_spec.ts @@ -1033,5 +1033,109 @@ runInEachFileSystem(() => { .toContain('ɵɵsetNgModuleScope(AppModule, { declarations: [App] }); })();'); }); }); + + describe('input transform', () => { + it('should generate input info for transform function imported externally using name', () => { + env.write('test.ts', ` + import {Component, NgModule, Input} from '@angular/core'; + import {externalFunc} from './some_where'; + + @Component({ + template: '{{x}}', + }) + export class Main { + @Input({transform: externalFunc}) + x: string = ''; + } + `); + + env.driveMain(); + const jsContents = env.getContents('test.js'); + + expect(jsContents).toContain('inputs: { x: ["x", "x", externalFunc] }'); + }); + + it('should generate input info for transform function imported externally using namespace', + () => { + env.write('test.ts', ` + import {Component, NgModule, Input} from '@angular/core'; + import * as n from './some_where'; + + @Component({ + template: '{{x}}', + }) + export class Main { + @Input({transform: n.externalFunc}) + x: string = ''; + } + `); + + env.driveMain(); + const jsContents = env.getContents('test.js'); + + expect(jsContents).toContain('inputs: { x: ["x", "x", n.externalFunc] }'); + }); + + it('should generate input info for transform function defined locally', () => { + env.write('test.ts', ` + import {Component, NgModule, Input} from '@angular/core'; + + @Component({ + template: '{{x}}', + }) + export class Main { + @Input({transform: localFunc}) + x: string = ''; + } + + function localFunc(value: string) { + return value; + } + `); + + env.driveMain(); + const jsContents = env.getContents('test.js'); + + expect(jsContents).toContain('inputs: { x: ["x", "x", localFunc] }'); + }); + + it('should generate input info for inline transform function', () => { + env.write('test.ts', ` + import {Component, NgModule, Input} from '@angular/core'; + + @Component({ + template: '{{x}}', + }) + export class Main { + @Input({transform: (v: string) => v + 'TRANSFORMED!'}) + x: string = ''; + } + `); + + env.driveMain(); + const jsContents = env.getContents('test.js'); + + expect(jsContents).toContain('inputs: { x: ["x", "x", (v) => v + \'TRANSFORMED!\'] }'); + }); + + it('should not check inline function param type', () => { + env.write('test.ts', ` + import {Component, NgModule, Input} from '@angular/core'; + + @Component({ + template: '{{x}}', + }) + export class Main { + @Input({transform: v => v + 'TRANSFORMED!'}) + x: string = ''; + } + `); + + env.driveMain(); + const jsContents = env.getContents('test.js'); + + expect(jsContents).toContain('inputs: { x: ["x", "x", v => v + \'TRANSFORMED!\'] }'); + }); + }); }); });