Problem
This is a small frontend frame for a React app that is built then served statically.
Because the app uses React-Router
for browser-based routing but the server doesn’t know anything about the app I added some code to the backend that reads the list of routes the app uses from a json file and creates dummy handlers for each that place a redirect=/path
cookie and redirect to the main page. That code is left out for brevity and to focus on the frontend.
Meanwhile, the Javascript React frontend in this question sets ups the app’s React-Router
routes based on the same external json file. The main page checks whether there is a redirect cookie set by the server and if so sends the user to that page.
The overall effect between the frontend and backend is that a user can go directly to app.com/widget/4
and without them knowing the server bounces them to the main page to start the browser-routing session and then back to the correct url
routes.json:
{
"routes": [
{"path":"/widget/:widgetId", "component": "WidgetContainer", "redirect": true},
{"path":"/", "component": "MainPageContainer", "exact": true}
]
}
index.js:
import React from 'react';
import ReactDOM from 'react-dom';
import { BrowserRouter as Router, Route } from "react-router-dom";
import ParseRoutes from "./Utils/ParseRoutes"
import { routes } from "./routes.json"
function AppContainer() {
const routeProps = ParseRoutes(routes)
return (
<Router>
{routeProps.map((props, i) => (<Route key={i} {...props} /> ))}
</Router>
)
}
ReactDOM.render(<AppContainer />, document.getElementById("root"))
ParseRoutes.js:
import MainPageContainer from "../Components/MainPage"
import WidgetContainer from "../Components/Widget"
const stringToComp = {
MainPageContainer: MainPageContainer,
WidgetContainer: WidgetContainer
}
const ParseRoutes = (routes) => (
routes.map((props) => ({...props, component: stringToComp[props.component]}))
)
export default ParseRoutes
RedirectIfRequested.js:
import React from "react"
import { Redirect } from "react-router-dom"
const getCookieValue = (a) => {
var b = document.cookie.match('(^|;)\s*' + a + '\s*=\s*([^;]+)')
return b ? b.pop() : ''
}
const RedirectIfRequested = () => {
const redirectTo = getCookieValue("redirect")
if ( redirectTo ) {
document.cookie = "redirect=''; max-age=-1"
return <Redirect to={redirectTo} />
}
return null
}
export default RedirectIfRequested
MainPage.js:
import React from "react"
import RedirectIfRequested from "../Utils/RedirectIfRequested.js"
export default function MainPageContainer(props) {
const redirect = RedirectIfRequested()
return redirect ? redirect : <MainPage />
}
function MainPage(props) {
return <div>Main Page</div>
}
Widget.js:
import React from "react"
import { useParams } from "react-router-dom"
export default function WidgetContainer(props) {
const id = useParams().widgetId
return <div> Widget {id} accessed via react-router </div>
}
Solution
In general, your code does a good job at naming variables/functions, and at splitting logic up in ways that make sense.
However, there are a few places where you are inconsistent with your code style – maybe using a linter would help out:
- In some files you indent with 2 spaces. Other files are indented with 4.
- There’s a number of import statements that have an extra space between the words. e.g.
import ParseRoutes from "./Utils/ParseRoutes"
A note on casing: Generally, we use TitleCase for classes and components only. camelCase is used for almost everything else. A file (or folder with an index.js) with a default export of a class or component may use TitleCase too. However, folders like “Util” or “Components” should probably all be in lowercase. Your “getCookieValue()” function is properly cased, but in the same file, your “RedirectIfRequested()” function is not – RedirectIfRequested is not a class or component. (there’s a couple other examples like that).
The local variables in getCookieValue() could probably be named better then “a” and “b”. Maybe rename “a” to “cookieKey” and “b” to “match”.
.pop() is not a common way to get a group from a regex. If you want to get the second group, just say that with code: match[2]
. Note that since you don’t actually care about the value of the first group in your regex, you can use a non-capturing group instead (the syntax is (?:...)
). For example, if your regex is a(b|B)c(d|D)e
, then b/B is group 1 and d/D is group 2, but if its a(?:b|B)c(d|D)e
, then you can’t get b/B, and d/D is group 1. You don’t have to use non-capture groups, but I just want to make sure you know that that’s an option.
Finally, and most importantly, all of this routing logic you made is actually not needed. The proper way to configure a server to handle single-page-application (SPA) routing is as follows:
Lets assume your SPA exits at example.com/my-app
, and your app has routes such as example.com/my-app/
(the home page) and example.com/my-app/widget/2
. You need to configure your server to return the same react bundle no matter which route they hit, as long as its within example.com/my-app
. So, going to example.com/my-app/does-not-exist
will give you the same javascript bundle as going to example.com/my-app/widget/2
. (Note that this is not the same as a redirection). Finally, javascript can check which URL you used (was it /my-app/does-not-exist/ or /my-app/widget/2) and route based on that information – something that react-router will do for you. If the route doesn’t exist, just show a 404 page from your React app.