From 48b68b221a42907dae43a2b28cd9ce58f194c41d Mon Sep 17 00:00:00 2001
From: William Chargin <wchargin@gmail.com>
Date: Wed, 3 Oct 2018 12:14:07 -0700
Subject: [PATCH] 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
---
 src/app/Link.js      | 17 ++++++++++++-----
 src/app/Link.test.js |  9 +++++++++
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/src/app/Link.js b/src/app/Link.js
index 4e60603..d73edef 100644
--- a/src/app/Link.js
+++ b/src/app/Link.js
@@ -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'.");
+    }
   }
 }
 
diff --git a/src/app/Link.test.js b/src/app/Link.test.js
index 33ae496..ca535b5 100644
--- a/src/app/Link.test.js
+++ b/src/app/Link.test.js
@@ -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", () => {