ReactJS paginated component with React Router

Question

I've recently started journey with ReactJS and to begin with I wanted to try something a bit more complex - paginated list of elements with back functionality enabled. Below are two main components that I'd like to get feedback about. Especially PagedListings.

import './App.css';
import PagedListings from './components/PagedListings';
import ScrollToTop from './components/ScrollToTop';
import React from "react";
import { Route } from 'react-router';
import { BrowserRouter as Router } from 'react-router-dom';
function App() {
 return (
 <Router>
 <ScrollToTop>
 <Route path={["/", "/:page"]} component={PagedListings}/>
 </ScrollToTop>
 </Router>
 );
}
export default App;

And the actual component executing planned task

import React, { useMemo } from "react";
import ListingsContainer from "./ListingsContainer";
import Pagination from "@material-ui/lab/Pagination";
import { useHistory, useLocation } from "react-router-dom";
const LISTINGS = ["Rec1", "Rec2", "Rec3",
 "Rec4", "Rec5", "Rec6", "Rec7", "Rec8",
 "Rec9", "Rec10",];
const DEFAULT_PAGE = 1;
const MAX_PAGE_COUNT = 10;
function PagedListings(props) {
 const history = useHistory();
 const location = useLocation();
 const urlParams = useMemo(() => {
 return new URLSearchParams(location.search);
 }, [location.search]);
 const handleChange = (
 event: React.MouseEvent<HTMLButtonElement> | null,
 newPage: number
 ) => {
 let oneBasedNewPage = newPage;
 urlParams.set("page", oneBasedNewPage.toString());
 updateURL();
 };
 const updateURL = () => {
 history.push({
 pathname: location.pathname,
 search: `?${urlParams}`,
 });
 };
 function getPageNumber() {
 let newPage = parseInt(urlParams.get("page")) || DEFAULT_PAGE;
 return newPage <= MAX_PAGE_COUNT ? newPage : MAX_PAGE_COUNT;
 }
 return (
 <>
 <ListingsContainer listings={LISTINGS.slice(0, getPageNumber())} />
 <Pagination
 count={MAX_PAGE_COUNT}
 defaultPage={DEFAULT_PAGE}
 page={getPageNumber()}
 onChange={handleChange}
 />
 </>
 );
}
export default PagedListings;

Answer 1

Your code looks quite reasonable. There are only a few small things I notice:

• Indent the JSX in App a bit more: ScrollToTop is a child of Router, so it should be indented further, not on the same level as Router. (It doesn't really make a difference here, but it's good to be consistent, and could be an improvement if that section ever got more complicated)

• Avoid let, prefer const - ESLint rule here.

• Or, in the case of oneBasedNewPage, consider defining the argument with that name instead of creating a new variable - and maybe call it oneIndexed, not oneBased:

  const handleChange = (
   event: React.MouseEvent<HTMLButtonElement> | null,
   oneIndexedNewPage: number
  ) => {
  

• URLSearchParams does not differentiate between numbers and strings.

  urlParams.set("page", oneBasedNewPage.toString());
  

  can be

  urlParams.set("page", oneIndexedNewPage);
  

  if you wish.

• Consider if using Math.min might be clearer than the conditional operator:

  function getPageNumber() {
   const newPage = parseInt(urlParams.get("page")) || DEFAULT_PAGE;
   return Math.min(newPage, MAX_PAGE_COUNT);
  }
  

• sebap123: Thank you for your review and all of the suggestions. I haven't been aware of the fact to use const instead of let in place you suggested.