refactor(forms): Move FormControl to an overridden exported constructor. (#44316) (#44806)

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 #44316

PR Close #44806
This commit is contained in:
Dylan Hunn
2021-11-30 14:46:46 -05:00
committed by Jessica Janiuk
parent cec158b23a
commit f0cfa00a34
6 changed files with 286 additions and 184 deletions
@@ -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 (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?`);
}
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 (ctorDocArray.length >= 2) {
throw new Error(`@overriddenImplementation found multiple docs for ${doc.overriddenImplementation}. You may only have one documented symbol for each.`);
}
}
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];
}
});
// 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;
}
});
}
@@ -39,15 +39,8 @@ 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];
@@ -55,7 +48,6 @@ describe('mergeOverriddenImplementation processor', () => {
switch(docName) {
case 'Foo': return [exportedNameDoc];
case 'FooCtor': return [{ctorProp: true, members: [{name: 'new'}]}];
case 'FooImpl': return [{implProp: true}];
}
};
@@ -63,8 +55,9 @@ describe('mergeOverriddenImplementation processor', () => {
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