API refactor for avoiding rendering props as DOM attributes

README.md

@@ -42,9 +42,11 @@ import React from 'react';
 import { NavLink } from 'react-router-dom';
 import withBreadcrumbs from 'react-router-breadcrumbs-hoc';
 
-// breadcrumbs can be any type of component or string
-const UserBreadcrumb = ({ match }) =>
-  <span>{match.params.userId}</span>; // use match param userId to fetch/display user name
+// breadcrumbs can be any type of component OR string. This userId
+// could be used to fetch & display the user's name, for example
+const UserBreadcrumb = ({ match }) => (
+  <span>{match.params.userId}</span>
+);
 
 // define some custom breadcrumbs for certain routes (optional)
 const routes = [
@@ -53,15 +55,16 @@ const routes = [
 ];
 
 // map & render your breadcrumb components however you want.
-// each `breadcrumb` has the props `key`, `location`, and `match` included!
 const Breadcrumbs = ({ breadcrumbs }) => (
   <div>
-    {breadcrumbs.map((breadcrumb, index) => (
-      <span key={breadcrumb.key}>
-        <NavLink to={breadcrumb.props.match.url}>
-          {breadcrumb}
-        </NavLink>
-        {(index < breadcrumbs.length - 1) && <i> / </i>}
+    {breadcrumbs.map(({
+      match,
+      breadcrumb
+      // other props are available during render, such as `location` 
+      // and any props found in your route objects will be included too
+    }) => (
+      <span key={match.url}>
+        <NavLink to={match.url}>{breadcrumb}</NavLink>
       </span>
     ))}
   </div>

src/index.js

@@ -30,30 +30,21 @@ const NO_BREADCRUMB = 'NO_BREADCRUMB';
  * with `match`, `location`, and `key` props.
  */
 const render = ({
-  /**
-   * extracting `component` here to avoid an invalid attribute warning
-   * see: https://github.com/icd2k3/react-router-breadcrumbs-hoc/issues/59
-   * This is actually a symptom of a larger issue with this current
-   * functionality of passing route data (needed for breadcrumb rendering)
-   * as props on the component itself. This has the unintended side-effect
-   * of rendering those props as element attributes in the DOM.
-   * TODO: Refactor this render logic (and probably the API) to not render
-   * those props as attributes on the breadcrumb element.
-   */
-  component,
-
+  component: reactRouterConfigComponent,
   breadcrumb,
   match,
   location,
   ...rest
 }) => {
   const componentProps = { match, location, key: match.url, ...rest };
-  if (typeof breadcrumb === 'function') {
-    return createElement(breadcrumb, componentProps);
-  }
-  return createElement('span', componentProps, breadcrumb);
-};
 
+  return {
+    ...componentProps,
+    breadcrumb: typeof breadcrumb === 'function'
+      ? createElement(breadcrumb, componentProps)
+      : createElement('span', { key: componentProps.key }, breadcrumb),
+  };
+};
 
 /**
  * Small helper method to get a default `humanize-string`
@@ -73,7 +64,7 @@ const getDefaultBreadcrumb = ({ pathSection, currentSection, location }) => {
  * Loops through the route array (if provided) and returns either a
  * user-provided breadcrumb OR a sensible default (if enabled) via `humanize-string`.
 */
-const getBreadcrumb = ({
+const getBreadcrumbMatch = ({
   currentSection,
   disableDefaults,
   excludePaths,
@@ -161,14 +152,14 @@ export const getBreadcrumbs = ({ routes, location, options = {} }) => {
     .replace(/\/$/, '')
     // Split pathname into sections.
     .split('/')
-    // Reduce over the sections and call `getBreadcrumb()` for each section.
+    // Reduce over the sections and call `getBreadcrumbMatch()` for each section.
     .reduce((previousSection, currentSection) => {
       // Combine the last route section with the currentSection.
       // For example, `pathname = /1/2/3` results in match checks for
       // `/1`, `/1/2`, `/1/2/3`.
       const pathSection = !currentSection ? '/' : `${previousSection}/${currentSection}`;
 
-      const breadcrumb = getBreadcrumb({
+      const breadcrumb = getBreadcrumbMatch({
         currentSection,
         location,
         pathSection,

src/index.test.js

@@ -14,8 +14,8 @@ import withBreadcrumbsCompiledUMD, { getBreadcrumbs as getBreadcrumbsCompiledUMD
 const components = {
   Breadcrumbs: ({ breadcrumbs }) => (
     <h1 className="breadcrumbs-container">
-      {breadcrumbs.map((breadcrumb, index) => (
-        <span key={breadcrumb.key}>
+      {breadcrumbs.map(({ breadcrumb, key }, index) => (
+        <span key={key}>
           {breadcrumb}
           {(index < breadcrumbs.length - 1) && <i> / </i>}
         </span>
@@ -29,6 +29,7 @@ const components = {
       {isLocationTest ? 'pass' : 'fail'}
     </span>
   ),
+  BreadcrumbExtraPropsTest: ({ foo, bar }) => <span>{foo}{bar}</span>,
 };
 
 const getHOC = () => {
@@ -78,7 +79,11 @@ const matchShape = {
 };
 
 components.Breadcrumbs.propTypes = {
-  breadcrumbs: PropTypes.arrayOf(PropTypes.node).isRequired,
+  breadcrumbs: PropTypes.arrayOf(PropTypes.shape({
+    breadcrumb: PropTypes.node.isRequired,
+    match: PropTypes.shape().isRequired,
+    location: PropTypes.shape.isRequired,
+  })).isRequired,
 };
 
 components.BreadcrumbMatchTest.propTypes = {
@@ -97,6 +102,11 @@ components.BreadcrumbLocationTest.propTypes = {
   }).isRequired,
 };
 
+components.BreadcrumbExtraPropsTest.propTypes = {
+  foo: PropTypes.string.isRequired,
+  bar: PropTypes.string.isRequired,
+};
+
 describe('react-router-breadcrumbs-hoc', () => {
   describe('Valid routes', () => {
     it('Should render breadcrumb components as expected', () => {
@@ -195,19 +205,6 @@ describe('react-router-breadcrumbs-hoc', () => {
       const { breadcrumbs } = render({ pathname: '/one/two/three', routes });
       expect(breadcrumbs).toBe('Home / One / TwoCustom / ThreeCustom');
     });
-
-    it('Should not produce a console warning for unsupported element attributes', () => {
-      // see: https://github.com/icd2k3/react-router-breadcrumbs-hoc/issues/59
-      global.console.error = jest.fn();
-      const routes = [
-        { path: '/one', breadcrumb: 'OneCustom', component: () => <span>One Page</span> },
-        { path: '/one/two', component: () => <span>Two Page</span> },
-      ];
-      const { breadcrumbs } = render({ pathname: '/one/two', routes });
-      expect(breadcrumbs).toBe('Home / OneCustom / Two');
-      // eslint-disable-next-line no-console
-      expect(console.error).not.toHaveBeenCalled();
-    });
   });
 
   describe('Defaults', () => {
@@ -268,13 +265,17 @@ describe('react-router-breadcrumbs-hoc', () => {
   });
 
   describe('When using additional props inside routes', () => {
-    it('Should forward additional from props from routes to generated breadcrumbs', () => {
-      const routes = [{ path: '/one', breadcrumb: 'One', foo: 'One Foo', bar: 'One Bar' }, { path: '/one/two', foo: 'Two Foo' }];
-      const breadcrumbs = getMethod()({ routes, location: { pathname: '/one/two' } });
-      expect(breadcrumbs[1].props).toHaveProperty('foo', 'One Foo');
-      expect(breadcrumbs[1].props).toHaveProperty('bar', 'One Bar');
-      expect(breadcrumbs[2].props).toHaveProperty('foo', 'Two Foo');
-      expect(breadcrumbs[2].props).not.toHaveProperty('bar');
+    it('Should pass through extra props to user-provided components', () => {
+      const routes = [
+        {
+          path: '/one',
+          breadcrumb: components.BreadcrumbExtraPropsTest,
+          foo: 'Pass through',
+          bar: ' props',
+        },
+      ];
+      const { breadcrumbs } = render({ pathname: '/one', routes });
+      expect(breadcrumbs).toBe('Home / Pass through props');
     });
   });
 
@@ -302,4 +303,11 @@ describe('react-router-breadcrumbs-hoc', () => {
         .toThrow('withBreadcrumbs: `path` must be provided in every route object');
     });
   });
+
+  describe('DOM rendering', () => {
+    it('Should not render props as element attributes on breadcrumbs', () => {
+      const { wrapper } = render({ pathname: '/one' });
+      expect(wrapper.html()).not.toContain('[object Object]');
+    });
+  });
 });