Code review #1

Entry-level project, ReactJS

Code review :

import React from 'react';

const CartContext = React.createContext({
  items: [],
  totalAmount: 0,
  addItem: (item) => {},
  removeItem: (id) => {}
});

export default CartContext;

I'd suggest you not to use React context as state management,

  1. Its not that popular as a state management
  2. there is some issues with performance in large scale

Good alternatives might be: recoil , react redux

for small-mid projects: zustand


Original Code

const Cart = (props) => {

Instead of writing 'props' I'd use:

const Cart = ({onClose}) => {

This will give me:

  1. a hint of what props Im getting
  2. Save me the need of writing ‘props.’ every time

Original Code

export default Cart;

My suggestion is NOT use default export for components, instead:

export const Cart = () => {

most of IDE will recognize the names and autocomplete or suggest the imports, it will keep you working cleaner and keep the same naming for those components


Original Code


const Cart = () => {
  const cartItems = (
    <ul className={classes['cart-items']}>
      {cartCtx.items.map((item) => (
        <CartItem
          key={item.id}
          name={item.name}
          amount={item.amount}
          price={item.price}
          onRemove={cartItemRemoveHandler.bind(null, item.id)}
          onAdd={cartItemAddHandler.bind(null, item)}
        />
      ))}
    </ul>
  );
 return (
  <div>
   {cartItems}
  </div>
 )
}

The issue I see here is that 'cartItems' component might be rendered a lot of times cause its a component inside arrow function component

so in that case you can do :

  1. use 'useMemo' hook -> it will help solve the not required renders
  2. take the cartItems and make it a standalone arrow function component => CartItems

Original Code

const CartIcon = () => {
  return (
    <svg
      xmlns='http://www.w3.org/2000/svg'
      viewBox='0 0 20 20'
      fill='currentColor'
    >
      <path d='M3 1a1 1 0 000 2h1.22l.305 1.222a.997.997 0 00.01.042l1.358 5.43-.893.892C3.74 11.846 4.632 14 6.414 14H15a1 1 0 000-2H6.414l1-1H14a1 1 0 00.894-.553l3-6A1 1 0 0017 3H6.28l-.31-1.243A1 1 0 005 1H3zM16 16.5a1.5 1.5 0 11-3 0 1.5 1.5 0 013 0zM6.5 18a1.5 1.5 0 100-3 1.5 1.5 0 000 3z' />
    </svg>
  );
};

if its a simple component better use, no need the return statement:

const CartIcon = () => (
    <svg
      xmlns='http://www.w3.org/2000/svg'
      viewBox='0 0 20 20'
      fill='currentColor'
    >
      <path d='M3 1a1 1 0 000 2h1.22l.305 1.222a.997.997 0 00.01.042l1.358 5.43-.893.892C3.74 11.846 4.632 14 6.414 14H15a1 1 0 000-2H6.414l1-1H14a1 1 0 00.894-.553l3-6A1 1 0 0017 3H6.28l-.31-1.243A1 1 0 005 1H3zM16 16.5a1.5 1.5 0 11-3 0 1.5 1.5 0 013 0zM6.5 18a1.5 1.5 0 100-3 1.5 1.5 0 000 3z' />
    </svg>
  );

Original Code:

    if (items.length === 0) {
      return;
    }
     setBtnIsHighlighted(true);
         .... more functionality

I like it to be more straight forward -> if I want to move on if Items.length is more than 0 => my code should reflect that:

    if (items.length !== 0) {
         setBtnIsHighlighted(true);
         .... more functionality
    }

project stracture: UI folder => what should be there? looks like utils so name it that way

inside that folder you have a lot of components with their styling make it sub-folders instead

card.js, card.module.css, Input.js, Input.module.css etc...

build it

card (folder), Input(folder), etc.. in each folder :

  1. index.js => the component
  2. style.module.css => the css

I don't like the use of css, even in convertion like module.css alternatives to try:

  1. scss / saas / less
  2. emotion/react
  3. styled-components
  4. tailwind

if you woul'd like to get a full code review regardless your project leave a comment bellow