Skip to content

Commit

Permalink
fix: dont pass all props to top level component
Browse files Browse the repository at this point in the history
  • Loading branch information
dpilch authored and alharris-at committed Oct 26, 2021
1 parent 9e02d28 commit ee9e1b4
Show file tree
Hide file tree
Showing 20 changed files with 419 additions and 270 deletions.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -1,25 +1,25 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Component Renderers BadgeRenderer 1`] = `"<Badge {...props} {...getOverrideProps(overrides, \\"Badge\\")}></Badge>"`;
exports[`Component Renderers BadgeRenderer 1`] = `"<Badge {...rest} {...getOverrideProps(overrides, \\"Badge\\")}></Badge>"`;

exports[`Component Renderers BoxRenderer 1`] = `"<View {...props} {...getOverrideProps(overrides, \\"View\\")}></View>"`;
exports[`Component Renderers BoxRenderer 1`] = `"<View {...rest} {...getOverrideProps(overrides, \\"View\\")}></View>"`;

exports[`Component Renderers ButtonRenderer 1`] = `"<Button {...props} {...getOverrideProps(overrides, \\"Button\\")}></Button>"`;
exports[`Component Renderers ButtonRenderer 1`] = `"<Button {...rest} {...getOverrideProps(overrides, \\"Button\\")}></Button>"`;

exports[`Component Renderers CardRenderer 1`] = `"<Card {...props} {...getOverrideProps(overrides, \\"Card\\")}></Card>"`;
exports[`Component Renderers CardRenderer 1`] = `"<Card {...rest} {...getOverrideProps(overrides, \\"Card\\")}></Card>"`;

exports[`Component Renderers CollectionRenderer 1`] = `"<Collection items={items || []} {...props} {...getOverrideProps(overrides, \\"Collection\\")}>{(item, index) => ()}</Collection>"`;
exports[`Component Renderers CollectionRenderer 1`] = `"<Collection items={items || []} {...rest} {...getOverrideProps(overrides, \\"Collection\\")}>{(item, index) => ()}</Collection>"`;

exports[`Component Renderers CustomComponentRenderer 1`] = `"<Custom {...findChildOverrides(props.overrides, \\"Custom\\")}></Custom>"`;

exports[`Component Renderers DividerRenderer 1`] = `"<Divider {...props} {...getOverrideProps(overrides, \\"Divider\\")}></Divider>"`;
exports[`Component Renderers DividerRenderer 1`] = `"<Divider {...rest} {...getOverrideProps(overrides, \\"Divider\\")}></Divider>"`;

exports[`Component Renderers FlexRenderer 1`] = `"<Flex {...props} {...getOverrideProps(overrides, \\"Flex\\")}></Flex>"`;
exports[`Component Renderers FlexRenderer 1`] = `"<Flex {...rest} {...getOverrideProps(overrides, \\"Flex\\")}></Flex>"`;

exports[`Component Renderers ImageRenderer 1`] = `"<Image {...props} {...getOverrideProps(overrides, \\"Image\\")}></Image>"`;
exports[`Component Renderers ImageRenderer 1`] = `"<Image {...rest} {...getOverrideProps(overrides, \\"Image\\")}></Image>"`;

exports[`Component Renderers StringRenderer basic 1`] = `"<>{\\"test\\"}</>"`;

exports[`Component Renderers StringRenderer missing props 1`] = `"Failed to render String - Unexpected component structure"`;

exports[`Component Renderers TextRenderer 1`] = `"<Text {...props} {...getOverrideProps(overrides, \\"Text\\")}>{\\"test\\"}</Text>"`;
exports[`Component Renderers TextRenderer 1`] = `"<Text {...rest} {...getOverrideProps(overrides, \\"Text\\")}>{\\"test\\"}</Text>"`;
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export default class BadgeRenderer extends ReactComponentWithChildrenRenderer<Ba
);

this.importCollection.addImport('@aws-amplify/ui-react', this.component.componentType);
this.importCollection.addImport('@aws-amplify/ui-react', `${this.component.componentType}Props`);

return element;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export default class ButtonRenderer extends ReactComponentWithChildrenRenderer<B
);

this.importCollection.addImport('@aws-amplify/ui-react', this.component.componentType);
this.importCollection.addImport('@aws-amplify/ui-react', `${this.component.componentType}Props`);

return element;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ export default class CardRenderer extends ReactComponentWithChildrenRenderer<Car
);

this.importCollection.addImport('@aws-amplify/ui-react', this.component.componentType);
this.importCollection.addImport('@aws-amplify/ui-react', `${this.component.componentType}Props`);

return element;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export default class CollectionRenderer extends ReactComponentWithChildrenRender
);

this.importCollection.addImport('@aws-amplify/ui-react', this.component.componentType);
this.importCollection.addImport('@aws-amplify/ui-react', `${this.component.componentType}Props`);

return element;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export default class CustomComponentRenderer extends ReactComponentWithChildrenR
);

this.importCollection.addImport('@aws-amplify/ui-react', this.component.componentType);
this.importCollection.addImport('@aws-amplify/ui-react', `${this.component.componentType}Props`);

return element;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export default class DividerRenderer extends ReactComponentRenderer<DividerProps
);

this.importCollection.addImport('@aws-amplify/ui-react', this.component.componentType);
this.importCollection.addImport('@aws-amplify/ui-react', `${this.component.componentType}Props`);

return element;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export default class FlexRenderer extends ReactComponentWithChildrenRenderer<Fle
);

this.importCollection.addImport('@aws-amplify/ui-react', this.component.componentType);
this.importCollection.addImport('@aws-amplify/ui-react', `${this.component.componentType}Props`);

return element;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ export default class ImageRenderer extends ReactComponentRenderer<ImageProps> {
);

this.importCollection.addImport('@aws-amplify/ui-react', this.component.componentType);
this.importCollection.addImport('@aws-amplify/ui-react', `${this.component.componentType}Props`);

return element;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ export default class TextRenderer extends ReactComponentRenderer<TextProps> {
);

this.importCollection.addImport('@aws-amplify/ui-react', this.component.componentType);
this.importCollection.addImport('@aws-amplify/ui-react', `${this.component.componentType}Props`);

return element;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export default class ViewRenderer extends ReactComponentWithChildrenRenderer<Vie
);

this.importCollection.addImport('@aws-amplify/ui-react', this.component.componentType);
this.importCollection.addImport('@aws-amplify/ui-react', `${this.component.componentType}Props`);

return element;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export abstract class ReactComponentRenderer<TPropIn> extends ComponentRendererB

private addPropsSpreadAttributes(attributes: JsxAttributeLike[]) {
if (this.node.isRoot()) {
const propsAttr = factory.createJsxSpreadAttribute(factory.createIdentifier('props'));
const propsAttr = factory.createJsxSpreadAttribute(factory.createIdentifier('rest'));
attributes.push(propsAttr);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export abstract class ReactComponentWithChildrenRenderer<TPropIn> extends Compon

private addPropsSpreadAttributes(attributes: JsxAttributeLike[]) {
if (this.node.isRoot()) {
const propsAttr = factory.createJsxSpreadAttribute(factory.createIdentifier('props'));
const propsAttr = factory.createJsxSpreadAttribute(factory.createIdentifier('rest'));
attributes.push(propsAttr);
}

Expand Down
123 changes: 82 additions & 41 deletions packages/studio-ui-codegen-react/lib/react-studio-template-renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ export abstract class ReactStudioTemplateRenderer extends StudioTemplateRenderer
}

renderBindingPropsType(component: StudioComponent): TypeAliasDeclaration {
const escapeHatchType = factory.createTypeLiteralNode([
const escapeHatchTypeNode = factory.createTypeLiteralNode([
factory.createPropertySignature(
undefined,
factory.createIdentifier('overrides'),
Expand All @@ -280,14 +280,43 @@ export abstract class ReactStudioTemplateRenderer extends StudioTemplateRenderer
undefined,
factory.createIntersectionTypeNode(
this.dropMissingListElements([
this.buildComponentPropNodes(component),
this.buildVariantPropNodes(component),
escapeHatchType,
this.buildPrimitivePropNode(component),
this.buildComponentPropNode(component),
this.buildVariantPropNode(component),
escapeHatchTypeNode,
]),
),
);
}

private hasPrimitivePropType(component: StudioComponent): boolean {
return component.componentType !== 'String';
}

private getParameterizedPrimitivePropType(component: StudioComponent): TypeNode | undefined {
switch (component.componentType) {
case 'Collection':
return factory.createTypeReferenceNode(factory.createIdentifier('CollectionProps'), [
factory.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword),
]);
default:
return undefined;
}
}

private buildPrimitivePropNode(component: StudioComponent): TypeNode | undefined {
if (!this.hasPrimitivePropType(component)) {
return undefined;
}

const parameterizedPrimitivePropType = this.getParameterizedPrimitivePropType(component);
const primitivePropType =
parameterizedPrimitivePropType ||
factory.createTypeReferenceNode(factory.createIdentifier(`${component.componentType}Props`), undefined);

return factory.createTypeReferenceNode(factory.createIdentifier('Partial'), [primitivePropType]);
}

/**
* This builder is responsible primarily for identifying the variant options, partioning them into
* required and optional parameters, then building the appropriate property signature based on that.
Expand All @@ -297,7 +326,7 @@ export abstract class ReactStudioTemplateRenderer extends StudioTemplateRenderer
size?: "large",
}
*/
private buildVariantPropNodes(component: StudioComponent): TypeNode | undefined {
private buildVariantPropNode(component: StudioComponent): TypeNode | undefined {
if (!isStudioComponentWithVariants(component)) {
return undefined;
}
Expand Down Expand Up @@ -349,7 +378,7 @@ export abstract class ReactStudioTemplateRenderer extends StudioTemplateRenderer
return factory.createTypeLiteralNode([...requiredProperties, ...optionalProperties]);
}

private buildComponentPropNodes(component: StudioComponent): TypeNode | undefined {
private buildComponentPropNode(component: StudioComponent): TypeNode | undefined {
const propSignatures: PropertySignature[] = [];
const bindingProps = component.bindingProperties;
if (bindingProps === undefined || !isStudioComponentWithBinding(component)) {
Expand Down Expand Up @@ -392,9 +421,8 @@ export abstract class ReactStudioTemplateRenderer extends StudioTemplateRenderer

private buildVariableStatements(component: StudioComponent): Statement[] {
const statements: Statement[] = [];

const elements: BindingElement[] = [];
if (isStudioComponentWithBinding(component)) {
const elements: BindingElement[] = [];
Object.entries(component.bindingProperties).forEach((entry) => {
const [propName, binding] = entry;
if (isSimplePropertyBinding(binding) || isDataPropertyBinding(binding)) {
Expand All @@ -407,34 +435,54 @@ export abstract class ReactStudioTemplateRenderer extends StudioTemplateRenderer
elements.push(bindingElement);
}
});
}

if (component.componentType === 'Collection') {
const bindingElement = factory.createBindingElement(
undefined,
undefined,
factory.createIdentifier('items'),
undefined,
);
elements.push(bindingElement);
}

const statement = factory.createVariableStatement(
if (component.componentType === 'Collection') {
const bindingElement = factory.createBindingElement(
undefined,
undefined,
factory.createIdentifier('items'),
undefined,
factory.createVariableDeclarationList(
[
factory.createVariableDeclaration(
factory.createObjectBindingPattern(elements),
undefined,
undefined,
factory.createIdentifier('props'),
),
],
ts.NodeFlags.Const,
),
);
statements.push(statement);
elements.push(bindingElement);
}

// remove overrides from rest of props
elements.push(
factory.createBindingElement(
undefined,
factory.createIdentifier('overrides'),
factory.createIdentifier('overridesProp'),
undefined,
),
);

// get rest of props to pass to top level component
elements.push(
factory.createBindingElement(
factory.createToken(ts.SyntaxKind.DotDotDotToken),
undefined,
factory.createIdentifier('rest'),
undefined,
),
);

const statement = factory.createVariableStatement(
undefined,
factory.createVariableDeclarationList(
[
factory.createVariableDeclaration(
factory.createObjectBindingPattern(elements),
undefined,
undefined,
factory.createIdentifier('props'),
),
],
ts.NodeFlags.Const,
),
);
statements.push(statement);

if (isStudioComponentWithVariants(component)) {
statements.push(this.buildVariantDeclaration(component.variants));
// TODO: In components, replace props.override with override (defined here).
Expand Down Expand Up @@ -542,8 +590,8 @@ export abstract class ReactStudioTemplateRenderer extends StudioTemplateRenderer
}

/**
* case: hasVariants = true => const overrides = { ...getOverridesFromVariants(variants), ...props.overrides };
* case: hasVariants = false => const overrides = { ...props.overrides };
* case: hasVariants = true => const overrides = { ...getOverridesFromVariants(variants, props) };
* case: hasVariants = false => const overrides = { ...overridesProp };
*/
private buildOverridesDeclaration(hasVariants: boolean): VariableStatement {
if (hasVariants) {
Expand Down Expand Up @@ -573,14 +621,7 @@ export abstract class ReactStudioTemplateRenderer extends StudioTemplateRenderer
]
: [],
)
.concat([
factory.createSpreadAssignment(
factory.createPropertyAccessExpression(
factory.createIdentifier('props'),
factory.createIdentifier('overrides'),
),
),
]),
.concat([factory.createSpreadAssignment(factory.createIdentifier('overridesProp'))]),
),
),
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,33 +57,34 @@ describe('Generated Components', () => {
});
});

describe('Conditional Data', () => {
it('Renders Button disabled when user is not logged in', () => {
cy.visit('http://localhost:3000/component-tests');
cy.get('[label="Disabled Conditional Button"]').get('[disabled]');
});

it('Renders May vote when user old enough', () => {
cy.visit('http://localhost:3000/component-tests');
cy.get('[label="May Vote Conditional Button"]').get('[prompt="May Vote, cast your vote."]');
});

it('Renders May not vote when user too young', () => {
cy.visit('http://localhost:3000/component-tests');
cy.get('[label="May Not Vote Conditional Button"]').get('[prompt="Sorry you cannot vote"]');
});
});

// TODO: We should be rendering the element as a text child, but concat doesn't seem to support that yet.
describe('Concatenated Data', () => {
it('Renders Button text as a concatenated, bound element', () => {
cy.visit('http://localhost:3000/component-tests');
cy.get('[label="Harry Callahan"]');
});

it('Renders Button text as a concatenated, bound element, with overrides', () => {
cy.visit('http://localhost:3000/component-tests');
cy.get('[label="Norm Gunderson"]');
describe('Generated Components', () => {
describe('Concatenated Data', () => {
it('Renders Button text as a concatenated, bound element', () => {
cy.visit('http://localhost:3000/component-tests');
cy.get('#concat-and-conditional').contains('Harry Callahan');
});

it('Renders Button text as a concatenated, bound element, with overrides', () => {
cy.visit('http://localhost:3000/component-tests');
cy.get('#concat-and-conditional').contains('Norm Gunderson');
});
});

describe('Conditional Data', () => {
it('Renders Button with one background when user is logged in', () => {
cy.visit('http://localhost:3000/component-tests');
cy.get('#concat-and-conditional').get('#conditional1').should('have.css', 'background-color', 'rgb(255, 0, 0)');
});

it('Renders Button with a different background when user is not logged in', () => {
cy.visit('http://localhost:3000/component-tests');
cy.get('#concat-and-conditional').get('#conditional2').should('have.css', 'background-color', 'rgb(0, 0, 255)');
});

it('Renders Button disabled when user is not logged in', () => {
cy.visit('http://localhost:3000/component-tests');
cy.get('#concat-and-conditional').get('#conditional2').get('[disabled]');
});
});
});

Expand Down
Loading

0 comments on commit ee9e1b4

Please sign in to comment.