fix(ngcc): do not output duplicate ɵprov properties (#34085)

Previously, the Angular AOT compiler would always add a
`ɵprov` to injectables. But in ngcc this resulted in duplicate `ɵprov`
properties since published libraries already have this property.

Now in ngtsc, trying to add a duplicate `ɵprov` property is an error,
while in ngcc the additional property is silently not added.

// FW-1750

PR Close #34085

aio/scripts/_payload-limits.json

@@ -12,7 +12,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 2987,
-        "main-es2015": 464734,
+        "main-es2015": 463671,
         "polyfills-es2015": 52503
       }
     }

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
       }

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,

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;
+}

packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts

@@ -32,7 +32,14 @@ export class InjectableDecoratorHandler implements
     DecoratorHandler<InjectableHandlerData, Decorator> {
   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({
+    const ɵprov = this.reflector.getMembersOfClass(node).find(member => member.name === 'ɵprov');
-      name: 'ɵprov',
+    if (ɵprov !== undefined && this.errorOnDuplicateProv) {
-      initializer: res.expression, statements,
+      throw new FatalDiagnosticError(
-      type: res.type,
+          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;
   }

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};
+}

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 {