refactor(forms): Move FormControl to an overridden exported construct…

…or. (angular#44316)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close angular#44316

aio/tools/transforms/angular-api-package/processors/mergeOverriddenImplementation.js

@@ -8,27 +8,29 @@
  * This looks like the following:
  * 
  * ```
+ * /**
+ * * @overriddenImplementation FooCtor
+ * *\/
  * export interface Foo {
  *   bar();
  * }
  * 
- * export class FooImpl {
- *   bar() {}
- * }
+ * type FooInterface = Foo;
+ * 
+ * export class FooImpl { ... }
  * 
  * export interface FooCtor {
  *   new(): Foo;
  * }
  * 
- * export const Foo: FooCtor = FooImpl as FooCtor;
+ * export const Foo: FooCtor =
+    (class Foo implements FooInterface { ... }
  * ```
  * 
- * This processor will correct the docs for symbol `Foo` by copying them over from `FooImpl`
- * to the exported symbol `Foo`. The processor will also copy all documented constructor overrides from `FooCtor`.
+ * This processor will extend the docs for symbol `Foo` by copying all documented constructor overrides from `FooCtor`.
  * 
- * In order to use this processor, annotate the exported constant with `@overriddenImplementation`,
- * and mark the implementation and constructor types as `@internal`. Place the desired
- * documentation on the implementation class.
+ * In order to use this processor, annotate the exported interface with `@overriddenImplementation`. Place the desired
+ * documentation on the interface and constructor signatures.
  */
  module.exports = function mergeOverriddenImplementation(getDocFromAlias, log) {
   return {
@@ -40,62 +42,33 @@
     ],
     $process(docs) {
       docs.forEach(doc => {
-        if (doc.overriddenImplementation) {          
-          // Check the AST is of the expected expression shape, and extract the identifiers.
-          const symbolAstObjects = [doc.declaration?.name, doc.declaration?.type, doc.declaration?.initializer?.expression];
-          if (symbolAstObjects.some(symbol => symbol === undefined)) {
-            throw new Error('@overriddenImplementation must have format `export const Foo: FooCtor = FooImpl as FooCtor;`');
-          }
-
-          // Convert the AST nodes into docs.
-          const symbolNames = symbolAstObjects.map(s => s.getText());
-          const symbolDocArrays = symbolNames.map(symbol => getDocFromAlias(symbol));
-          for (let i = 0; i < symbolDocArrays.length; i++) {
-            if (symbolDocArrays[i].length === 0) {
-              throw new Error(`@overriddenImplementation failed to find a doc for ${symbolNames[i]}. Are you sure this symbol is documented and exported?`);
-            }
-            if (symbolDocArrays[i].length >= 2) {
-              throw new Error(`@overriddenImplementation found multiple docs for ${symbolNames[i]}. You may only have one documented symbol for each.`);
+        if (doc.overriddenImplementation) {   
+          // Convert the specified name into a doc.
+          const ctorDocArray = getDocFromAlias(doc.overriddenImplementation);
+            if (ctorDocArray.length === 0) {
+              throw new Error(`@overriddenImplementation failed to find a doc for ${doc.overriddenImplementation}. Are you sure this symbol is documented and exported?`);
             }
-          }
-          const symbolDocs = symbolDocArrays.map(a => a[0]);
-          const exportedNameDoc = symbolDocs[0];
-          const ctorDoc = symbolDocs[1];
-          const implDoc = symbolDocs[2];
-
-          // Clean out the unwanted properties from the exported doc.
-          Object.keys(doc).forEach(key => {
-            if (!this.propertiesToKeep.includes(key)) {
-              delete doc[key];
+            if (ctorDocArray.length >= 2) {
+              throw new Error(`@overriddenImplementation found multiple docs for ${doc.overriddenImplementation}. You may only have one documented symbol for each.`);
             }
-          });
-
-          // Copy over all the properties from the implementation doc.
-          Object.keys(implDoc).forEach(key => {
-            if (!this.propertiesToKeep.includes(key)) {
-              exportedNameDoc[key] = implDoc[key];
-            }
-          });
+
+          const ctorDoc = ctorDocArray[0];
 
           // Copy the constructor overrides from the constructor doc, if any are present.
-          if (!ctorDoc.members || ctorDoc.members.length !== 1 || !ctorDoc.members[0].name.includes('new')) {
-            throw new Error(`@overriddenImplementation requires that the provided constructor ${symbolNames[1]} have exactly one member called "new", possibly with multiple overrides.`);
+          if (!ctorDoc.members || ctorDoc.members.length === 0 || !ctorDoc.members[0].name.includes('new')) {
+            throw new Error(`@overriddenImplementation requires that the provided constructor ${ctorDoc} have a member called "new", possibly with multiple overrides.`);
           }
-          exportedNameDoc.constructorDoc = ctorDoc.members[0];
+          doc.constructorDoc = ctorDoc.members[0];
 
-          // Mark symbols other than the exported name as internal.
+          // Mark the constructor doc internal.
           if (!ctorDoc.internal) {
-            log.warn(`Constructor doc ${symbolNames[1]} was not marked '@internal'; adding this annotation.`);
+            log.warn(`Constructor doc ${ctorDoc} was not marked '@internal'; adding this annotation.`);
             ctorDoc.internal = true;
           }
-          if (!implDoc.internal) {
-            log.warn(`Implementation doc ${symbolNames[2]} was not marked '@internal'; adding this annotation.`);
-            implDoc.internal = true;
-          }
 
-          // The exported doc should not be private, unlike the implementation doc.
-          exportedNameDoc.privateExport = false;
-          exportedNameDoc.internal = false;
+          // The exported doc should not be private.
+          doc.privateExport = false;
+          doc.internal = false;
         }
       });
     }

aio/tools/transforms/angular-api-package/processors/mergeOverriddenImplementation.spec.js

@@ -39,32 +39,25 @@ describe('mergeOverriddenImplementation processor', () => {
 
   it('should replace properties with those from the implementation and constructor docs', () => {
     const exportedNameDoc = {
-      overriddenImplementation: 'Foo has an overridden implementation.', // This processor should apply
-      declaration: { // Imitate a valid AST
-        name: {getText: () => 'Foo'},
-        type: {getText: () => 'FooCtor'},
-        initializer: {
-          expression: {getText: () => 'FooImpl'},
-        },
-      },
-      exportedProp: true, // This prop will be removed
+      overriddenImplementation: 'FooCtor', // This processor should apply
+      exportedProp: true, // Documentation on the exported interface will remain
     };
     const docs = [exportedNameDoc];
 
     const fakeGetDocs = (docName) => {
       switch(docName) {
         case 'Foo': return [exportedNameDoc];
         case 'FooCtor': return [{ctorProp: true, members: [{name: 'new'}]}];
-        case 'FooImpl': return [{implProp: true}];
       }
     };
 
     getDocFromAlias.and.callFake(fakeGetDocs);
     processor.$process(docs);
 
     expect(docs).toEqual([{
-      // Property copied from the implementation
-      implProp: true,
+      overriddenImplementation: 'FooCtor',
+      // Original documentation property
+      exportedProp: true,
       // Constructor signature property
       constructorDoc: {name: 'new'},
       // The exported symbol should be explicitly marked non-internal

goldens/public-api/forms/forms.md

@@ -204,10 +204,8 @@ export class DefaultValueAccessor extends BaseControlValueAccessor implements Co
 // @public
 export class EmailValidator extends AbstractValidatorDirective {
     email: boolean | string;
-
     // (undocumented)
     enabled(input: boolean): boolean;
-
     // (undocumented)
     static ɵdir: i0.ɵɵDirectiveDeclaration<EmailValidator, "[email][formControlName],[email][formControl],[email][ngModel]", never, { "email": "email"; }, {}, never>;
     // (undocumented)
@@ -297,8 +295,7 @@ export class FormBuilder {
 }
 
 // @public
-export class FormControl extends AbstractControl {
-    constructor(formState?: any, validatorOrOpts?: ValidatorFn | ValidatorFn[] | FormControlOptions | null, asyncValidator?: AsyncValidatorFn | AsyncValidatorFn[] | null);
+export interface FormControl extends AbstractControl {
     readonly defaultValue: any;
     patchValue(value: any, options?: {
         onlySelf?: boolean;
@@ -320,6 +317,9 @@ export class FormControl extends AbstractControl {
     }): void;
 }
 
+// @public (undocumented)
+export const FormControl: θFormControlCtor;
+
 // @public
 export class FormControlDirective extends NgControl implements OnChanges, OnDestroy {
     constructor(validators: (Validator | ValidatorFn)[], asyncValidators: (AsyncValidator | AsyncValidatorFn)[], valueAccessors: ControlValueAccessor[], _ngModelWarningConfig: string | null);
@@ -765,6 +765,13 @@ export class Validators {
 // @public (undocumented)
 export const VERSION: Version;
 
+// @public
+export interface θFormControlCtor {
+    new (): FormControl;
+    new (formState: any, validatorOrOpts?: ValidatorFn | ValidatorFn[] | FormControlOptions | null, asyncValidator?: AsyncValidatorFn | AsyncValidatorFn[] | null): FormControl;
+    prototype: FormControl;
+}
+
 // (No @packageDocumentation comment for this package)
 
 ```

packages/forms/src/forms.ts

@@ -17,7 +17,6 @@
  * explicitly.
  */
 
-
 export {ɵInternalFormsSharedModule} from './directives';
 export {AbstractControlDirective} from './directives/abstract_control_directive';
 export {AbstractFormGroupDirective} from './directives/abstract_form_group_directive';
@@ -43,7 +42,7 @@ export {NgSelectOption, SelectControlValueAccessor} from './directives/select_co
 export {SelectMultipleControlValueAccessor, ɵNgSelectMultipleOption} from './directives/select_multiple_control_value_accessor';
 export {AsyncValidator, AsyncValidatorFn, CheckboxRequiredValidator, EmailValidator, MaxLengthValidator, MaxValidator, MinLengthValidator, MinValidator, PatternValidator, RequiredValidator, ValidationErrors, Validator, ValidatorFn} from './directives/validators';
 export {FormBuilder} from './form_builder';
-export {AbstractControl, AbstractControlOptions, FormArray, FormControl, FormControlOptions, FormControlStatus, FormGroup} from './model';
+export {AbstractControl, AbstractControlOptions, FormArray, FormControl, FormControlOptions, FormControlStatus, FormGroup, θFormControlCtor} from './model';
 export {NG_ASYNC_VALIDATORS, NG_VALIDATORS, Validators} from './validators';
 export {VERSION} from './version';