From a1973728e6a01bc48e2ef87cf3c9006912f4da96 Mon Sep 17 00:00:00 2001 From: PAkerstrand Date: Tue, 1 Nov 2016 14:43:24 +0100 Subject: [PATCH 1/2] Adds support for async routes by not only taking "routes", but also the matched "components" into account --- src/RedialContext.js | 2 +- src/triggerHooks.js | 2 +- src/util/findRouteByComponent.js | 31 ++++++++++++------------- src/util/mapKeys.js | 4 ++-- tests/util/findRouteByComponent.test.js | 17 ++++++++++---- tests/util/mapKeys.test.js | 4 ++-- 6 files changed, 34 insertions(+), 26 deletions(-) diff --git a/src/RedialContext.js b/src/RedialContext.js index 838ca62..0ba6e42 100755 --- a/src/RedialContext.js +++ b/src/RedialContext.js @@ -9,7 +9,7 @@ import getAllComponents from './getAllComponents'; function hydrate(renderProps) { if (typeof __REDIAL_PROPS__ !== 'undefined' && Array.isArray(__REDIAL_PROPS__)) { - const getMapKeyForComponent = createMapKeys(renderProps.routes); + const getMapKeyForComponent = createMapKeys(renderProps.routes, renderProps.components); const components = getAllComponents(renderProps.components); const componentKeys = components.map(getMapKeyForComponent); return createMap(componentKeys, __REDIAL_PROPS__); diff --git a/src/triggerHooks.js b/src/triggerHooks.js index 85a3d8f..f4c3823 100644 --- a/src/triggerHooks.js +++ b/src/triggerHooks.js @@ -31,7 +31,7 @@ export default function triggerHooks({ const getProps = (key) => () => redialMap.get(key) || {}; - const getMapKeyForComponent = createMapKeys(renderProps.routes); + const getMapKeyForComponent = createMapKeys(renderProps.routes, renderProps.components); const completeLocals = (component) => { const key = getMapKeyForComponent(component); diff --git a/src/util/findRouteByComponent.js b/src/util/findRouteByComponent.js index d229a54..a15b762 100644 --- a/src/util/findRouteByComponent.js +++ b/src/util/findRouteByComponent.js @@ -1,24 +1,23 @@ -export default function findRouteByComponent(component, routes) { +export default function findRouteByComponent(component, routes, components) { const result = {}; - for (const route of routes) { - if (route.component === component) { - result.route = route; - return result; - } - if (route.components) { - const foundNamedComponent = Object.keys(route.components) + components.some((matchedComponent, i) => { + if (typeof matchedComponent === 'object') { + Object.keys(matchedComponent) .some(key => { - const found = route.components[key] === component; - if (found) { + if (matchedComponent[key] === component) { result.name = key; + matchedComponent = matchedComponent[key]; // eslint-disable-line no-param-reassign + return true; } - return found; + return false; }); - if (foundNamedComponent) { - result.route = route; - return result; - } } - } + if (component === matchedComponent) { + result.route = routes[i]; + return true; + } + return false; + }); + return result; } diff --git a/src/util/mapKeys.js b/src/util/mapKeys.js index c28b2dd..d60c893 100644 --- a/src/util/mapKeys.js +++ b/src/util/mapKeys.js @@ -1,9 +1,9 @@ import findRouteByComponent from './findRouteByComponent'; import getRoutePath from './getRoutePath'; -export default function createGenerateMapKeyByMatchedRoutes(routes) { +export default function createGenerateMapKeyByMatchedRoutes(routes, components) { return (component) => { - const { route, name } = findRouteByComponent(component, routes); + const { route, name } = findRouteByComponent(component, routes, components); if (!route) { throw new Error('`component` not found among the matched `routes`'); } diff --git a/tests/util/findRouteByComponent.test.js b/tests/util/findRouteByComponent.test.js index e2b32d2..98c489c 100644 --- a/tests/util/findRouteByComponent.test.js +++ b/tests/util/findRouteByComponent.test.js @@ -2,10 +2,14 @@ import expect from 'expect'; import findRouteByComponent from '../../src/util/findRouteByComponent'; +function toMatchedComponents(routes) { + return routes.map(route => route.component || route.components); +} + describe('findRouteByComponent', () => { const component = () => {}; it('Returns an empty object for empty routes', () => { - expect(findRouteByComponent(component, [])).toEqual({}); + expect(findRouteByComponent(component, [], [])).toEqual({}); }); it('Returns an empty object when the component cannot be found among the routes', () => { const routes = [ @@ -16,7 +20,7 @@ describe('findRouteByComponent', () => { component: function andNotThisOnceEither() {}, }, ]; - expect(findRouteByComponent(component, routes)).toEqual({}); + expect(findRouteByComponent(component, routes, toMatchedComponents(routes))).toEqual({}); }); it('Returns the first matched route', () => { const routes = [ @@ -27,7 +31,9 @@ describe('findRouteByComponent', () => { component, }, ]; - expect(findRouteByComponent(component, routes)).toEqual({ route: routes[1] }); + expect(findRouteByComponent(component, routes, toMatchedComponents(routes))).toEqual({ + route: routes[1], + }); }); it('Handles `components` for named routes', () => { const routes = [ @@ -43,6 +49,9 @@ describe('findRouteByComponent', () => { }, }, ]; - expect(findRouteByComponent(component, routes)).toEqual({ route: routes[1], name: 'c' }); + expect(findRouteByComponent(component, routes, toMatchedComponents(routes))).toEqual({ + route: routes[1], + name: 'c', + }); }); }); diff --git a/tests/util/mapKeys.test.js b/tests/util/mapKeys.test.js index 4649a62..7557781 100644 --- a/tests/util/mapKeys.test.js +++ b/tests/util/mapKeys.test.js @@ -39,7 +39,7 @@ describe('mapKeys', () => { ]; describe('regular routes', () => { - const mapKeyByComponent = createGenerateMapKeyByMatchedRoutes(routes); + const mapKeyByComponent = createGenerateMapKeyByMatchedRoutes(routes, routes.map(route => route.component || route.components)); it('Throws an Error when the component cannot be found among the matched routes', () => { expect(() => mapKeyByComponent(() => {}, routes)).toThrow( @@ -55,7 +55,7 @@ describe('mapKeys', () => { }); describe('named routes', () => { - const mapKeyByComponent = createGenerateMapKeyByMatchedRoutes(namedRoutes); + const mapKeyByComponent = createGenerateMapKeyByMatchedRoutes(namedRoutes, namedRoutes.map(route => route.component || route.components)); it('Gives the path up to the matched route', () => { expect(mapKeyByComponent(namedRoutes[0].component, namedRoutes)).toBe('/'); expect(mapKeyByComponent(namedRoutes[1].component, namedRoutes)).toBe('//'); From 1dacfc7078cfd194adf0c0b88bfae08241f13e4e Mon Sep 17 00:00:00 2001 From: PAkerstrand Date: Wed, 2 Nov 2016 13:42:58 +0100 Subject: [PATCH 2/2] Re-wrote `findRouteByComponent` to make intent clearer --- src/util/findRouteByComponent.js | 44 ++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/src/util/findRouteByComponent.js b/src/util/findRouteByComponent.js index a15b762..8a6b253 100644 --- a/src/util/findRouteByComponent.js +++ b/src/util/findRouteByComponent.js @@ -1,22 +1,34 @@ -export default function findRouteByComponent(component, routes, components) { +import isPlainObject from 'lodash.isplainobject'; + +const isNamedComponents = isPlainObject; + +function searchNamedComponents(component, namedComponents, correspondingRoute, result) { + return Object.keys(namedComponents) + .some(name => { + const isMatch = namedComponents[name] === component; + if (isMatch) { + /* eslint-disable no-param-reassign */ + result.name = name; + result.route = correspondingRoute; + /* eslint-enable no-param-reassign */ + } + return isMatch; + }); +} + +export default function findRouteByComponent(component, matchedRoutes, matchedComponents) { const result = {}; - components.some((matchedComponent, i) => { - if (typeof matchedComponent === 'object') { - Object.keys(matchedComponent) - .some(key => { - if (matchedComponent[key] === component) { - result.name = key; - matchedComponent = matchedComponent[key]; // eslint-disable-line no-param-reassign - return true; - } - return false; - }); + + matchedComponents.some((matchedComponent, i) => { + if (isNamedComponents(matchedComponent)) { + return searchNamedComponents(component, matchedComponent, matchedRoutes[i], result); } - if (component === matchedComponent) { - result.route = routes[i]; - return true; + + const isMatch = component === matchedComponent; + if (isMatch) { + result.route = matchedRoutes[i]; } - return false; + return isMatch; }); return result;