thetrevorharmon / gatsby-theme-shopify-manager

The easiest way to start a Shopify shop on Gatsby.
https://gatsby-theme-shopify-manager.netlify.app/
MIT License
121 stars 11 forks source link

Memoize hook functions #59

Closed nandorojo closed 4 years ago

nandorojo commented 4 years ago

Currently, the useful hooks look like this:

import { useCartItems } from './useCartItems';

export function useGetLineItem() {
  const cartItems = useCartItems();

  function getLineItem(variantId: string | number): ShopifyBuy.LineItem | null {
    if (cartItems.length < 1) {
      return null;
    }

    const item = cartItems.find((cartItem) => {
      // eslint-disable-next-line @typescript-eslint/ban-ts-ignore
      // @ts-ignore
      return cartItem.variant.id === variantId;
    });

    if (item == null) {
      return null;
    }

    return item;
  }

  return getLineItem;
}

I'd like to propose changing them to this:

import { useCallback } from 'react'
import { useCartItems } from './useCartItems';

export function useGetLineItem() {
  const cartItems = useCartItems();

  const getLineItem = useCallback((variantId: string | number): ShopifyBuy.LineItem | null => {
    if (cartItems.length < 1) {
      return null;
    }

    const item = cartItems.find((cartItem) => {
      // eslint-disable-next-line @typescript-eslint/ban-ts-ignore
      // @ts-ignore
      return cartItem.variant.id === variantId;
    });

    if (item == null) {
      return null;
    }

    return item;
  }, [cartItems])

  return getLineItem;
}

This is a performance optimization that makes it easier to pass them around to components, to trigger fewer re-renders.

What do you think? I'm happy to help with a PR if you'd prefer.

thetrevorharmon commented 4 years ago

Is there a specific use case to describe the times where it’s causing re-renders? I’m not against the idea, but I don’t want to optimize/memorize too soon or unnecessarily.

nandorojo commented 4 years ago

I actually got around it, so I'll close this for now.

nandorojo commented 4 years ago

Calling a function in an effect is a use case, as seen here: https://github.com/thetrevorharmon/gatsby-theme-shopify-manager/issues/60