diff --git a/aio/scripts/_payload-limits.json b/aio/scripts/_payload-limits.json index a6e33d1f10..07ad2dff4d 100755 --- a/aio/scripts/_payload-limits.json +++ b/aio/scripts/_payload-limits.json @@ -12,7 +12,7 @@ "master": { "uncompressed": { "runtime-es2015": 2987, - "main-es2015": 464734, + "main-es2015": 463671, "polyfills-es2015": 52503 } } diff --git a/integration/_payload-limits.json b/integration/_payload-limits.json index 81ff687a8d..899cd7079a 100644 --- a/integration/_payload-limits.json +++ b/integration/_payload-limits.json @@ -39,7 +39,7 @@ "master": { "uncompressed": { "runtime-es2015": 2289, - "main-es2015": 267979, + "main-es2015": 267389, "polyfills-es2015": 36808, "5-es2015": 751 } @@ -49,7 +49,7 @@ "master": { "uncompressed": { "runtime-es2015": 2289, - "main-es2015": 227232, + "main-es2015": 226528, "polyfills-es2015": 36808, "5-es2015": 779 } diff --git a/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts b/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts index 473d8a4734..83ccdeae2e 100644 --- a/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts +++ b/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts @@ -96,7 +96,7 @@ export class DecorationAnalyzer { this.isCore, /* annotateForClosureCompiler */ false), new InjectableDecoratorHandler( this.reflectionHost, NOOP_DEFAULT_IMPORT_RECORDER, this.isCore, - /* strictCtorDeps */ false), + /* strictCtorDeps */ false, /* errorOnDuplicateProv */ false), new NgModuleDecoratorHandler( this.reflectionHost, this.evaluator, this.fullMetaReader, this.fullRegistry, this.scopeRegistry, this.referencesRegistry, this.isCore, /* routeAnalyzer */ null, diff --git a/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts b/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts index 96ab23dc36..23dbc31670 100644 --- a/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts +++ b/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts @@ -175,6 +175,36 @@ runInEachFileSystem(() => { expect(jsContents).toMatch(/\bvar _c0 =/); }); + it('should add ɵfac but not duplicate ɵprov properties on injectables', () => { + compileIntoFlatEs5Package('test-package', { + '/index.ts': ` + import {Injectable, ɵɵdefineInjectable} from '@angular/core'; + export const TestClassToken = 'TestClassToken'; + @Injectable({providedIn: 'module'}) + export class TestClass { + static ɵprov = ɵɵdefineInjectable({ factory: () => {}, token: TestClassToken, providedIn: "module" }); + } + `, + }); + + const before = fs.readFile(_(`/node_modules/test-package/index.js`)); + const originalProp = /ɵprov[^;]+/.exec(before) ![0]; + mainNgcc({ + basePath: '/node_modules', + targetEntryPointPath: 'test-package', + propertiesToConsider: ['main'], + }); + const after = fs.readFile(_(`/node_modules/test-package/index.js`)); + + expect(before).toContain(originalProp); + expect(countOccurrences(before, 'ɵprov')).toEqual(1); + expect(countOccurrences(before, 'ɵfac')).toEqual(0); + + expect(after).toContain(originalProp); + expect(countOccurrences(after, 'ɵprov')).toEqual(1); + expect(countOccurrences(after, 'ɵfac')).toEqual(1); + }); + it('should add generic type for ModuleWithProviders and generate exports for private modules', () => { compileIntoApf('test-package', { @@ -1231,3 +1261,12 @@ runInEachFileSystem(() => { } }); }); + +function countOccurrences(haystack: string, needle: string): number { + const regex = new RegExp(needle, 'g'); + let count = 0; + while (regex.exec(haystack)) { + count++; + } + return count; +} diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts b/packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts index 50550a3ed0..a55662ddd6 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts @@ -32,7 +32,14 @@ export class InjectableDecoratorHandler implements DecoratorHandler { constructor( private reflector: ReflectionHost, private defaultImportRecorder: DefaultImportRecorder, - private isCore: boolean, private strictCtorDeps: boolean) {} + private isCore: boolean, private strictCtorDeps: boolean, + /** + * What to do if the injectable already contains a ɵprov property. + * + * If true then an error diagnostic is reported. + * If false then there is no error and a new ɵprov property is not added. + */ + private errorOnDuplicateProv = true) {} readonly precedence = HandlerPrecedence.SHARED; @@ -93,11 +100,18 @@ export class InjectableDecoratorHandler implements results.push(factoryRes); } - results.push({ - name: 'ɵprov', - initializer: res.expression, statements, - type: res.type, - }); + const ɵprov = this.reflector.getMembersOfClass(node).find(member => member.name === 'ɵprov'); + if (ɵprov !== undefined && this.errorOnDuplicateProv) { + throw new FatalDiagnosticError( + ErrorCode.INJECTABLE_DUPLICATE_PROV, ɵprov.nameNode || ɵprov.node || node, + 'Injectables cannot contain a static ɵprov property, because the compiler is going to generate one.'); + } + + if (ɵprov === undefined) { + // Only add a new ɵprov if there is not one already + results.push({name: 'ɵprov', initializer: res.expression, statements, type: res.type}); + } + return results; } diff --git a/packages/compiler-cli/src/ngtsc/annotations/test/injectable_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/test/injectable_spec.ts new file mode 100644 index 0000000000..29b2134f84 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/annotations/test/injectable_spec.ts @@ -0,0 +1,87 @@ +/** + * @license + * Copyright Google Inc. 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 {ErrorCode, FatalDiagnosticError, ngErrorCode} from '../../diagnostics'; +import {absoluteFrom} from '../../file_system'; +import {runInEachFileSystem} from '../../file_system/testing'; +import {NOOP_DEFAULT_IMPORT_RECORDER} from '../../imports'; +import {TypeScriptReflectionHost, isNamedClassDeclaration} from '../../reflection'; +import {getDeclaration, makeProgram} from '../../testing'; +import {InjectableDecoratorHandler} from '../src/injectable'; + +runInEachFileSystem(() => { + describe('InjectableDecoratorHandler', () => { + describe('compile()', () => { + it('should produce a diagnostic when injectable already has a static ɵprov property (with errorOnDuplicateProv true)', + () => { + const {handler, TestClass, ɵprov, analysis} = + setupHandler(/* errorOnDuplicateProv */ true); + try { + handler.compile(TestClass, analysis); + return fail('Compilation should have failed'); + } catch (err) { + if (!(err instanceof FatalDiagnosticError)) { + return fail('Error should be a FatalDiagnosticError'); + } + const diag = err.toDiagnostic(); + expect(diag.code).toEqual(ngErrorCode(ErrorCode.INJECTABLE_DUPLICATE_PROV)); + expect(diag.file.fileName.endsWith('entry.ts')).toBe(true); + expect(diag.start).toBe(ɵprov.nameNode !.getStart()); + } + }); + + it('should not add new ɵprov property when injectable already has one (with errorOnDuplicateProv false)', + () => { + const {handler, TestClass, ɵprov, analysis} = + setupHandler(/* errorOnDuplicateProv */ false); + const res = handler.compile(TestClass, analysis); + expect(res).not.toContain(jasmine.objectContaining({name: 'ɵprov'})); + }); + }); + + }); +}); + +function setupHandler(errorOnDuplicateProv: boolean) { + const ENTRY_FILE = absoluteFrom('/entry.ts'); + const ANGULAR_CORE = absoluteFrom('/node_modules/@angular/core/index.d.ts'); + const {program} = makeProgram([ + { + name: ANGULAR_CORE, + contents: 'export const Injectable: any; export const ɵɵdefineInjectable: any', + }, + { + name: ENTRY_FILE, + contents: ` + import {Injectable, ɵɵdefineInjectable} from '@angular/core'; + export const TestClassToken = 'TestClassToken'; + @Injectable({providedIn: 'module'}) + export class TestClass { + static ɵprov = ɵɵdefineInjectable({ factory: () => {}, token: TestClassToken, providedIn: "module" }); + }` + }, + ]); + const checker = program.getTypeChecker(); + const reflectionHost = new TypeScriptReflectionHost(checker); + const handler = new InjectableDecoratorHandler( + reflectionHost, NOOP_DEFAULT_IMPORT_RECORDER, /* isCore */ false, + /* strictCtorDeps */ false, errorOnDuplicateProv); + const TestClass = getDeclaration(program, ENTRY_FILE, 'TestClass', isNamedClassDeclaration); + const ɵprov = reflectionHost.getMembersOfClass(TestClass).find(member => member.name === 'ɵprov'); + if (ɵprov === undefined) { + throw new Error('TestClass did not contain a `ɵprov` member'); + } + const detected = handler.detect(TestClass, reflectionHost.getDecoratorsOfDeclaration(TestClass)); + if (detected === undefined) { + throw new Error('Failed to recognize TestClass'); + } + const {analysis} = handler.analyze(TestClass, detected.metadata); + if (analysis === undefined) { + throw new Error('Failed to analyze TestClass'); + } + return {handler, TestClass, ɵprov, analysis}; +} diff --git a/packages/compiler-cli/src/ngtsc/diagnostics/src/code.ts b/packages/compiler-cli/src/ngtsc/diagnostics/src/code.ts index 592c12059c..9e9c7cd8ae 100644 --- a/packages/compiler-cli/src/ngtsc/diagnostics/src/code.ts +++ b/packages/compiler-cli/src/ngtsc/diagnostics/src/code.ts @@ -94,6 +94,11 @@ export enum ErrorCode { * No matching pipe was found for a */ MISSING_PIPE = 8004, + + /** + * An injectable already has a `ɵprov` property. + */ + INJECTABLE_DUPLICATE_PROV = 9001 } export function ngErrorCode(code: ErrorCode): number {