link: remove `styles` attribute from child (#911)

Summary:
By using `<a {...this.props}>{children}</a>`, we were forwarding the
Aphrodite selectors as `styles`. This caused the static HTML for the
page to include `<a styles="[object Object]">`, which is annoying.

Test Plan:
Unit tests extended: they fail before this change and pass after it.
Also clicked a router link and an external link in the application.

wchargin-branch: link-child-styles
This commit is contained in:
William Chargin 2018-10-03 12:14:07 -07:00 committed by GitHub
parent 16ed92549a
commit 48b68b221a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 21 additions and 5 deletions

View File

@ -30,16 +30,23 @@ type LinkProps = $ReadOnly<{
}>;
export default class Link extends Component<LinkProps> {
render() {
const linkClass = css(styles.link, this.props.styles);
const {styles: customStyles, children, ...rest} = this.props;
const linkClass = css(styles.link, customStyles);
const className = this.props.className
? `${linkClass} ${this.props.className}`
: linkClass;
const Tag = "to" in this.props ? RouterLink : "a";
return (
<Tag {...this.props} className={className}>
{this.props.children}
const make = (Tag) => (
<Tag {...rest} className={className}>
{children}
</Tag>
);
if ("to" in this.props) {
return make(RouterLink);
} else if ("href" in this.props) {
return make("a");
} else {
throw new Error("Must specify either 'to' or 'href'.");
}
}
}

View File

@ -45,6 +45,14 @@ describe("src/app/Link", () => {
expect(typeof element.prop("className")).toBe("string");
});
it("fails if neither `to` nor `href` is provided", () => {
// $ExpectFlowError
const component = <Link>uhhhhh</Link>;
expect(() => {
shallow(component);
}).toThrow("Must specify either 'to' or 'href'.");
});
it("has deterministic className", () => {
const e1 = shallow(<Link href="#" />);
const e2 = shallow(<Link href="#" />);
@ -55,6 +63,7 @@ describe("src/app/Link", () => {
const e1 = shallow(<Link href="#" />);
const e2 = shallow(<Link href="#" styles={[styles.x]} />);
expect(e2.prop("className")).not.toEqual(e1.prop("className"));
expect(e2.props()).not.toHaveProperty("styles");
});
it("forwards class name", () => {