Skip to content

Commit

Permalink
refactor(forms): Move FormControl to an overridden exported construct…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
dylhunn committed Jan 26, 2022
1 parent 80467c3 commit 600377c
Show file tree
Hide file tree
Showing 6 changed files with 286 additions and 133 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@
* }
*
* export const Foo: FooCtor = FooImpl as FooCtor;
*
* /**
* * @overriddenImplementation FooCtor
* *\/
* export type Foo = FooImpl;
* ```
*
* This processor will correct the docs for symbol `Foo` by copying them over from `FooImpl`
Expand All @@ -40,15 +45,28 @@
],
$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;`');
if (doc.overriddenImplementation) {
// Check the AST is of the expected expression shape, and extract the identifiers: exported name, ctor, and impl
const symbolNames = [doc.name, doc.overriddenImplementation, doc.typeDefinition];
if (symbolNames.some(symbol => symbol === undefined)) {
throw new Error(`@overriddenImplementation ${doc.overriddenImplementation} in incorrect format. Should be:
/**
* @overriddenImplementation FooCtor
*/
export interface Foo { ... }
but got:
/**
* @overriddenImplementation ${doc.overriddenImplementation}
*/
export interface ${doc.name}
`);
}

// 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) {
Expand Down Expand Up @@ -78,8 +96,8 @@
});

// 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 ${symbolNames[1]} have a member called "new", possibly with multiple overrides.`);
}
exportedNameDoc.constructorDoc = ctorDoc.members[0];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,9 @@ 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'},
},
},
name: 'Foo',
overriddenImplementation: 'FooCtor', // This processor should apply
typeDefinition: 'FooImpl',
exportedProp: true, // This prop will be removed
};
const docs = [exportedNameDoc];
Expand All @@ -63,6 +58,7 @@ describe('mergeOverriddenImplementation processor', () => {
processor.$process(docs);

expect(docs).toEqual([{
name: 'Foo',
// Property copied from the implementation
implProp: true,
// Constructor signature property
Expand Down
15 changes: 11 additions & 4 deletions goldens/public-api/forms/forms.md
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,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)
Expand Down Expand Up @@ -298,8 +296,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;
Expand All @@ -321,6 +318,16 @@ export class FormControl extends AbstractControl {
}): void;
}

// @public
export const FormControl: FormControlCtor;

// @public
export interface FormControlCtor {
new (): FormControl;
new (formState: any, validatorOrOpts?: ValidatorFn | ValidatorFn[] | FormControlOptions | null, asyncValidator?: AsyncValidatorFn | AsyncValidatorFn[] | null): FormControl;
prototype: FormControl;
}

// @public
export class FormControlDirective extends NgControl implements OnChanges, OnDestroy {
constructor(validators: (Validator | ValidatorFn)[], asyncValidators: (AsyncValidator | AsyncValidatorFn)[], valueAccessors: ControlValueAccessor[], _ngModelWarningConfig: string | null);
Expand Down
3 changes: 1 addition & 2 deletions packages/forms/src/forms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
* explicitly.
*/


export {ɵInternalFormsSharedModule} from './directives';
export {AbstractControlDirective} from './directives/abstract_control_directive';
export {AbstractFormGroupDirective} from './directives/abstract_form_group_directive';
Expand All @@ -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, FormControlCtor, FormControlOptions, FormControlStatus, FormGroup} from './model';
export {NG_ASYNC_VALIDATORS, NG_VALIDATORS, Validators} from './validators';
export {VERSION} from './version';

Expand Down
Loading

0 comments on commit 600377c

Please sign in to comment.