2
\$\begingroup\$

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>
}
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

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.

\$\endgroup\$
4
  • \$\begingroup\$ The regex variables/pop was the one piece I copied pasted from stack overflow and has already been fixed since the question was posted. Do you have any resource or know what even the right terms to google would be to workout which react chunks should be served when? Writing JavaScript is fine but whenever babel/webpack/building it comes up 'yarn run build' is about all I know \$\endgroup\$
    – Coupcoup
    Dec 23, 2020 at 21:24
  • \$\begingroup\$ reactjs.org/docs/code-splitting.html Is this the direction you're talking about for splitting bundles? \$\endgroup\$
    – Coupcoup
    Dec 24, 2020 at 1:17
  • 1
    \$\begingroup\$ @Coupcoup code-splitting is a good practice, but it's not needed to fix the issue your code was trying to fix. Try the following link: create-react-app.dev/docs/deployment/… - I don't know if you use create-react-app or not, but the link should contain relevant information either way. once you have that fixed, and if you're also wanting to do code splitting to make page-loads quicker, than yes, the link you have is a great resource. \$\endgroup\$ Dec 24, 2020 at 16:39
  • \$\begingroup\$ Thanks for the pointer! That was actually an incredibly easy change (like replacing one function call) to once it clicked to serve index.html instead of redirect to it \$\endgroup\$
    – Coupcoup
    Dec 24, 2020 at 18:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.