From 42ffbe376de7a10e082249daaafb4378a0f58c6c Mon Sep 17 00:00:00 2001 From: Christian Musa Date: Fri, 29 May 2020 13:48:47 -0300 Subject: [PATCH 1/2] fix: use react-router match route method This solves a bug where the route matching couldn't find the correct route when the route was using dynamic parameters (ie: /:something/blue). --- packages/client/src/Before.component.jsx | 52 ++++-------------------- packages/client/src/ensureReady.js | 2 +- packages/client/src/utils.js | 8 ++-- 3 files changed, 13 insertions(+), 49 deletions(-) diff --git a/packages/client/src/Before.component.jsx b/packages/client/src/Before.component.jsx index 9703a9f..95a3806 100644 --- a/packages/client/src/Before.component.jsx +++ b/packages/client/src/Before.component.jsx @@ -12,22 +12,8 @@ import type { } from 'Before.component'; import React, { useCallback, useEffect, useReducer, useRef, useMemo, memo } from 'react'; import { withRouter, Switch, Route, type ContextRouter } from 'react-router-dom'; -import { - compose, - concat, - find, - has, - head, - ifElse, - identity, - last, - propOr, - prop, - propEq, - split, - useWith -} from 'ramda'; -import { getQueryString } from './utils'; +import { compose, concat, has, head, ifElse, last, propOr, prop, split } from 'ramda'; +import { getQueryString, findRouteByPathname } from './utils'; /** * Extract the base path from given full pathname, for example given the following url `/foo?bar=2` @@ -36,12 +22,7 @@ import { getQueryString } from './utils'; * @param {string} pathname the pathname to retrieve the pathname * @returns {string} the base path */ -const getBasePath: (pathname: string) => string = compose( - head, - split('#'), - head, - split('?') -); +const getBasePath: (pathname: string) => string = compose(head, split('#'), head, split('?')); /** * Extract the search part of a given full pathname or window.Location, for example given the following url `/foo?bar=2` @@ -53,28 +34,9 @@ const getBasePath: (pathname: string) => string = compose( const getSearch: (pathname: string | LocationType) => string = ifElse( has('search'), prop('search'), - compose( - concat('?'), - last, - split('?') - ) + compose(concat('?'), last, split('?')) ); -/** - * Retrieve the current route by a given path. - * @func - * @param {string} pathname - * @param {array} routes an array of route to filter - * @returs {object|undefined} a valid route - **/ -const getRouteByPathname: (path: string, routes: Array) => ?AsyncRoute = useWith(find, [ - compose( - propEq('path'), - getBasePath - ), - identity -]); - /** * Generates a random string * @func @@ -156,8 +118,8 @@ export function Before(props: BeforeComponentWithRouterProps) { const createHistoryMethod = useCallback( (name: string) => (obj: string | LocationType, state?: { [key: string]: string }) => { - const path: string = propOr(obj, 'pathname', obj); - const route = getRouteByPathname(path, routes); + const path: string = getBasePath(propOr(obj, 'pathname', obj)); + const route = findRouteByPathname(path, routes); if (route) { const search = getSearch(obj); fetchInitialProps( @@ -194,7 +156,7 @@ export function Before(props: BeforeComponentWithRouterProps) { interrupt.current = action === 'POP'; if (disableInitialPropsCache || !initialProps.current[location.pathname]) { // This solves a weird case when, on an advanced step of the flow, the user does a browser back - const route = getRouteByPathname(location.pathname, routes); + const route = findRouteByPathname(location.pathname, routes); if (route) { fetchInitialProps( route, diff --git a/packages/client/src/ensureReady.js b/packages/client/src/ensureReady.js index 08463cc..ab944e0 100644 --- a/packages/client/src/ensureReady.js +++ b/packages/client/src/ensureReady.js @@ -24,7 +24,7 @@ export async function loadCurrentRoute( pathname: string = window.location.pathname ) { let data; - const route = findRouteByPathname(pathname)(routes); + const route = findRouteByPathname(pathname, routes); if (route) { const match = matchPath(pathname, route); diff --git a/packages/client/src/utils.js b/packages/client/src/utils.js index 8853eca..5887b92 100644 --- a/packages/client/src/utils.js +++ b/packages/client/src/utils.js @@ -1,7 +1,7 @@ // @flow strict import { parse } from 'query-string'; import { matchPath } from 'react-router-dom'; -import { complement, find, isNil } from 'ramda'; +import { complement, find, isNil, curry } from 'ramda'; import type { QueryType } from 'Before.component'; import type { Route } from 'ensureReady'; @@ -51,8 +51,10 @@ const checkMatchPath = (pathname: string) => (route: Route) => isNotNil(matchPat /** * Returns a function that will find a route by a given request pathname. - * @func + * @function * @param {string} pathname a request pathname * @returns {function} (routes[]) => route */ -export const findRouteByPathname = (pathname: string) => find(checkMatchPath(pathname)); +export const findRouteByPathname = curry((pathname: string, routes: Array) => + find(checkMatchPath(pathname), routes) +); From a63f3f755449f261113becd995e881cf892d1550 Mon Sep 17 00:00:00 2001 From: Christian Musa Date: Fri, 29 May 2020 13:52:02 -0300 Subject: [PATCH 2/2] docs: update jsdocs for the find route method --- packages/client/src/utils.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/client/src/utils.js b/packages/client/src/utils.js index 5887b92..b90e9cf 100644 --- a/packages/client/src/utils.js +++ b/packages/client/src/utils.js @@ -53,8 +53,9 @@ const checkMatchPath = (pathname: string) => (route: Route) => isNotNil(matchPat * Returns a function that will find a route by a given request pathname. * @function * @param {string} pathname a request pathname - * @returns {function} (routes[]) => route + * @param {Array} routes a list of routes + * @returns {Route} the route matched with the pathname or undefined */ -export const findRouteByPathname = curry((pathname: string, routes: Array) => +export const findRouteByPathname = curry((pathname: string, routes: Array) => find(checkMatchPath(pathname), routes) );