REST countries API [react, react router v6, redux, asyncThunks]

Community feedback

• 

  j0sephh123•140

  @j0sephh123Posted about 2 years ago

  • When you need to use logic in react components, but don't need to return JSX, as is in src/components/ScrollToTop/ScrollToTop.jsx - create a custom hook
  • Haven't used unwrap for createAsyncThunk, so thanks for introducing that to me :)
  • src/routes/app/App.jsx useEffect seems to be missing dependencies. I suggest that you setup eslint and the hooks plugin - https://www.npmjs.com/package/eslint-plugin-react-hooks It is pain to setup, but will warn you for missing or incorrect dependencies for useEffect, useMemo, useCallback and so on.
  • Regarding your struggle with styles - you can look into SCSS modules, Tailwind or https://stitches.dev/. Really depends on your style. Personally I'm using SCSS modules at work and working on a project here with stitches and it is very nice - very convenient for a global theme. A bit hard to get into.
  • src/components/Header/Header.jsx - you can create enum for themes, instead of using raw strings
  • src/components/InputElement/InputElement.jsx - 'await' has no effect on the type of this expression this is for setTimeout. Also, you can use the shorthand syntax for this: onChange={handleInput} Also shorthand for this: {countryName && <div className="close" onClick={handleErase}></div>} It is good that you have 1sec debounce after typing to avoid sending fast requests. You can also use https://usehooks.com/useDebounce/ or extract your own logic into a custom hook for reusability. (not that it is needed here)
    • src/features/regions/Regions.jsx - hidden={countryRegion === '' ? true : false} can be simply hidden={countryRegion === ''}
    • same for selected={countryRegion === region ? true : false}
    • what is firstRender used for? It doesn't seem to do anything, you just set it in the useEffect.
    • src/features/countries/Countries.jsx In handleScroll avoid lenghty statements inside if. Create a variable and use it inside if. I will try to provide an idea. Note that there is a high probability that this syntax may not be working outside of the box, it is just to illustrate what I have in mind:

  // Countries
  const Countries = () => {
    ...
  const isLoading = useSelector(selectCountriesIsLoading);
  useHandleScroll(isLoading, () =>
    dispatch(
      loadCountries({ countryName: null, countryRegion: null, scroll: true })
    )
  );
    ...
  

  And a new hook file

  //src/features/countries/useHandleScroll.js
  export default function useHandleScroll(isLoading, callback) {
  
  const handleScroll = () => {
  	const ourVariableName = window.innerHeight + Math.ceil(window.pageYOffset) >= document.body.offsetHeight && !isLoading;
  
  	if (ourVariableName) {
  		callback();
  	}
  };
  
  useEffect(() => {
  	window.addEventListener("scroll", handleScroll);
  		return () => window.removeEventListener("scroll", handleScroll);
  	}, [isLoading]);
  }
  

  Marked as helpful

  1

  ---

  

  Daniil•170

  @Daniil034Posted about 2 years ago

  @j0sephh123 Oh my god, I haven't thought anyone would actually go through my code. I'm ashamed ((( Yeah, it's bad.

  src/routes/app/App.jsx - it doesn't miss dependencies, I just forgot to discard those braces);

  firstRender is needed because without it the page would reload after 1 second on first load and after every return on main page from country page)

  All other suggestions noted. Really, you taught me a lot)

  Especially thank you for introducing custom hooks because I was going to use them in my next project along with all other thing I have in mind (as well as tool for working with css, maybe it will be scss as you said).

  You are awesome, dude)

  1

  

  j0sephh123•140

  @j0sephh123Posted about 2 years ago

  @Daniil034 There is nothing to be ashamed of. You have created something that is looking good and working. Of course, there are things that could be better, but with time and experience, you will refine these things.

  1