feat: Improve link API (4Catalyzer#241)

BREAKING CHANGE: Rename Component prop on <Link> to as
BREAKING CHANGE: Remove childProps prop from <Link>

README.md

@@ -723,7 +723,7 @@ const link1 = (
 
 const link2 = (
   <Link
-    Component={CustomAnchor}
+    as={CustomAnchor}
     to={{
       pathname: '/widgets/bar',
       query: { the: query },
@@ -738,16 +738,22 @@ const link2 = (
 `<Link>` accepts the following props:
 
 - `to`: a [location descriptor](https://github.com/4Catalyzer/farce#locations-and-location-descriptors) for the link's destination
+- `exact`: if specified, the link will only render as active if the current location exactly matches the `to` location descriptor; by default, the link also will render as active on subpaths of the `to` location descriptor
 - `activeClassName`: if specified, a CSS class to append to the component's CSS classes when the link is active
 - `activeStyle`: if specified, a style object to append merge with the component's style object when the link is active
 - `activePropName`: if specified, a prop to inject with a boolean value with the link's active state
-- `exact`: if specified, the link will only render as active if the current location exactly matches the `to` location descriptor; by default, the link also will render as active on subpaths of the `to` location descriptor
-- `Component`: if specified, the custom element type to use for the link; by default, the link will render an `<a>` element
-
-`<Link>` forwards additional props to the child element. If you need to pass in additional props to the custom link component that collide with the names of props used by `<Link>`, specify the optional `childProps` prop as an object containing those props.
+- `as`: if specified, the custom element type to use for the link; by default, the link will render an `<a>` element
 
 A link will navigate per its `to` location descriptor when clicked. You can prevent this navigation by providing an `onClick` handler that calls `event.preventDefault()`.
 
+`<Link>` accepts a function for `children`. If `children` is a function, then `<Link>` will render the return value of that function, and will ignore `activeClassName`, `activeStyle`, `activePropName`, and `as` above. The function will be called with an object with the following properties:
+
+- `href`: the URL for the link
+- `active`: whether the link is active
+- `onClick`: the click event handler for the link element
+
+Otherwise, `<Link>` forwards additional props to the child element.
+
 If you have your own store with `foundReducer` installed on a key other than `found`, use `createConnectedLink` with a options object with a `getFound` function to create a custom link component class, as with `createConnectedRouter` above.
 
 #### Programmatic navigation

package.json

@@ -51,6 +51,8 @@
     "collectCoverageFrom": [
       "src/**"
     ],
+    "resetMocks": true,
+    "restoreMocks": true,
     "setupFiles": [
       "<rootDir>/test/setup.js"
     ],

src/BaseLink.js

@@ -5,7 +5,7 @@ import warning from 'warning';
 import { routerShape } from './PropTypes';
 
 const propTypes = {
-  Component: PropTypes.elementType,
+  as: PropTypes.elementType,
   to: PropTypes.oneOfType([PropTypes.string, PropTypes.object]).isRequired,
   match: PropTypes.object.isRequired,
   activeClassName: PropTypes.string,
@@ -15,11 +15,11 @@ const propTypes = {
   exact: PropTypes.bool,
   target: PropTypes.string,
   onClick: PropTypes.func,
-  childProps: PropTypes.object, // In case of name conflicts here.
+  children: PropTypes.oneOfType([PropTypes.node, PropTypes.func]),
 };
 
 const defaultProps = {
-  Component: 'a',
+  as: 'a',
   exact: false,
 };
 
@@ -56,33 +56,50 @@ class BaseLink extends React.Component {
 
   render() {
     const {
-      Component,
+      as: Component,
       to,
       match,
       activeClassName,
       activeStyle,
       activePropName,
       router,
       exact,
-      childProps,
       ...props
     } = this.props;
 
-    if (__DEV__ && props.component) {
-      warning(
-        typeof Component === 'function',
-        'Link to %s with `component` prop `%s` has an element type that ' +
-          'is not a component. The expected prop for the link component is ' +
-          '`Component`.',
-        JSON.stringify(to),
-        props.component.displayName || props.component.name,
-      );
+    if (__DEV__ && typeof Component !== 'function') {
+      for (const wrongPropName of ['component', 'Component']) {
+        const wrongPropValue = props[wrongPropName];
+        if (!wrongPropValue) {
+          continue;
+        }
+
+        warning(
+          false,
+          'Link to %s with `%s` prop `%s` has an element type that is not a component. The expected prop for the link component is `as`.',
+          JSON.stringify(to),
+          wrongPropName,
+          wrongPropValue.displayName || wrongPropValue.name || 'UNKNOWN',
+        );
+      }
     }
 
-    if (activeClassName || activeStyle || activePropName) {
+    const href = router.createHref(to);
+    const childrenIsFunction = typeof props.children === 'function';
+
+    if (
+      childrenIsFunction ||
+      activeClassName ||
+      activeStyle ||
+      activePropName
+    ) {
       const toLocation = router.createLocation(to);
       const active = router.isActive(match, toLocation, { exact });
 
+      if (childrenIsFunction) {
+        return props.children({ href, active, onClick: this.onClick });
+      }
+
       if (active) {
         if (activeClassName) {
           props.className = props.className
@@ -103,8 +120,7 @@ class BaseLink extends React.Component {
     return (
       <Component
         {...props}
-        {...childProps}
-        href={router.createHref(to)}
+        href={href}
         onClick={this.onClick} // This overrides props.onClick.
       />
     );

test/BaseLink.test.js

@@ -136,11 +136,11 @@ describe('<BaseLink>', () => {
       router.isActive.mockReturnValueOnce(true);
       const link = mount(
         <BaseLink
+          as={CustomComponent}
           to="/"
+          activePropName="active"
           match={{}}
           router={router}
-          Component={CustomComponent}
-          activePropName="active"
         />,
       );
 
@@ -151,11 +151,11 @@ describe('<BaseLink>', () => {
       router.isActive.mockReturnValueOnce(false);
       const link = mount(
         <BaseLink
+          as={CustomComponent}
           to="/"
+          activePropName="active"
           match={{}}
           router={router}
-          Component={CustomComponent}
-          activePropName="active"
         />,
       );
 

test/Link-warnings.test.js

@@ -0,0 +1,61 @@
+jest.mock('warning');
+
+import React from 'react';
+import warning from 'warning';
+
+import Link from '../src/Link';
+import { mountWithRouter } from './helpers';
+
+const CustomComponent = () => <div />;
+
+describe('<Link> warnings', () => {
+  it('should warn on component prop', async () => {
+    // The below will log a warning for an invalid prop.
+    jest.spyOn(console, 'error').mockImplementation(() => {});
+
+    await mountWithRouter(<Link component={CustomComponent} to="/" />);
+
+    expect(warning).toHaveBeenCalledWith(
+      false,
+      'Link to %s with `%s` prop `%s` has an element type that is not a component. The expected prop for the link component is `as`.',
+      '"/"',
+      'component',
+      'CustomComponent',
+    );
+  });
+
+  it('should warn on Component prop', async () => {
+    // The below will log a warning for an invalid prop.
+    jest.spyOn(console, 'error').mockImplementation(() => {});
+
+    await mountWithRouter(<Link Component={CustomComponent} to="/" />);
+
+    expect(warning).toHaveBeenCalledWith(
+      false,
+      'Link to %s with `%s` prop `%s` has an element type that is not a component. The expected prop for the link component is `as`.',
+      '"/"',
+      'Component',
+      'CustomComponent',
+    );
+  });
+
+  it('should not warn when as prop is specified', async () => {
+    await mountWithRouter(
+      <Link
+        as={CustomComponent}
+        to="/"
+        component={CustomComponent}
+        // eslint-disable-next-line react/jsx-no-duplicate-props
+        Component={CustomComponent}
+      />,
+    );
+
+    expect(warning).not.toHaveBeenCalledWith(
+      false,
+      'Link to %s with `%s` prop `%s` has an element type that is not a component. The expected prop for the link component is `as`.',
+      expect.anything(),
+      expect.anything(),
+      expect.anything(),
+    );
+  });
+});

test/Link.test.js

@@ -13,10 +13,47 @@ describe('<Link>', () => {
 
   it('should support a custom component', async () => {
     const link = await mountWithRouter(
-      <Link Component={CustomComponent} to="/" />,
+      <Link
+        as={CustomComponent}
+        to="/"
+        activePropName="active"
+        otherProp="foo"
+      />,
     );
     expect(link.find('a')).toHaveLength(0);
-    expect(link.find(CustomComponent)).toHaveLength(1);
+
+    const customNode = link.find(CustomComponent);
+    expect(customNode).toHaveLength(1);
+    expect(customNode.props()).toEqual({
+      href: '/',
+      active: true,
+      onClick: expect.any(Function),
+      otherProp: 'foo',
+    });
+  });
+
+  it('should support functional children', async () => {
+    const link = await mountWithRouter(
+      <Link to="/" otherProp="foo">
+        {({ href, active, onClick, props }) => (
+          <CustomComponent
+            {...props}
+            href={href}
+            active={active}
+            onClick={onClick}
+          />
+        )}
+      </Link>,
+    );
+    expect(link.find('a')).toHaveLength(0);
+
+    const customNode = link.find(CustomComponent);
+    expect(customNode).toHaveLength(1);
+    expect(customNode.props()).toEqual({
+      href: '/',
+      active: true,
+      onClick: expect.any(Function),
+    });
   });
 
   describe('active state', () => {

types/example-app/AppPage.tsx

@@ -1,7 +1,13 @@
+import Link from 'found/lib/Link';
 import * as React from 'react';
-import { Link } from 'found';
 
-function AppPage({ children }: { children: React.ReactNode }) {
+import { CustomLink } from './CustomLink';
+
+interface Props {
+  children: React.ReactNode;
+}
+
+export function AppPage({ children }: Props) {
   return (
     <div>
       <ul>
@@ -11,15 +17,22 @@ function AppPage({ children }: { children: React.ReactNode }) {
           </Link>
         </li>
         <li>
-          <Link to="/widgets/foo" activeClassName="active">
+          <Link as={CustomLink} to="/widgets/foo" activePropName="active">
             Foo widget
           </Link>
         </li>
+        <li>
+          <Link to="/widgets/bar">
+            {({ href, active, onClick }) => (
+              <CustomLink href={href} active={active} onClick={onClick}>
+                Bar widget
+              </CustomLink>
+            )}
+          </Link>
+        </li>
       </ul>
 
       {children}
     </div>
   );
 }
-
-export { AppPage };

types/example-app/ComponentWithRouter.tsx

@@ -0,0 +1,24 @@
+import { WithRouterProps } from 'found';
+import withRouter from 'found/lib/withRouter';
+import * as React from 'react';
+
+interface Props extends WithRouterProps {
+  foo?: boolean;
+}
+
+class ComponentWithRouter extends React.Component<Props> {
+  render() {
+    const {
+      match: { location },
+      router,
+    } = this.props;
+    return (
+      <div>
+        {location.pathname}, {router}
+      </div>
+    );
+  }
+}
+
+const ComponentWithRouterContainer = withRouter(ComponentWithRouter);
+export { ComponentWithRouterContainer as ComponentWithRouter };

types/example-app/CustomLink.tsx

@@ -0,0 +1,16 @@
+import * as React from 'react';
+
+interface Props {
+  href: string;
+  active: boolean;
+  onClick: (event: React.SyntheticEvent<any>) => void;
+  children: React.ReactNode;
+}
+
+export function CustomLink({ href, active, onClick, children }: Props) {
+  return (
+    <a href={href} className={active ? 'active' : ''} onClick={onClick}>
+      {children}
+    </a>
+  );
+}

types/example-app/MainPage.tsx

@@ -1,9 +1,11 @@
 import * as React from 'react';
-import { WithRouterComponent } from './WithRouterComponent';
 
-const MainPage = () => (
-  <div>
-    <WithRouterComponent />
-  </div>
-);
-export { MainPage };
+import { ComponentWithRouter } from './ComponentWithRouter';
+
+export function MainPage() {
+  return (
+    <div>
+      <ComponentWithRouter />
+    </div>
+  );
+}

types/example-app/WidgetsPage.tsx

@@ -1,4 +1,5 @@
 import * as React from 'react';
 
-const WidgetsPage = () => <div />;
-export { WidgetsPage };
+export function WidgetsPage() {
+  return <div />;
+}