Esri / jsapi-resources

A collection of resources for developers using the ArcGIS Maps SDK for JavaScript.
https://developers.arcgis.com/javascript/latest/
Apache License 2.0
708 stars 561 forks source link

React - there are cases where Viewmodels are mutating their inputs #494

Closed jonathandawe-geollect closed 11 months ago

jonathandawe-geollect commented 11 months ago

User Error 🤦‍♂️

I was making a basic mistake where an object inside a dependency array was causing infinite re-renders as described here.

IMPORTANT

Actual behavior

I am using the ArcGIS JS API (v.4.28) with React and have created a custom React hook so that I can utilise the Basemap toggle view model logic in another section of my application. The issue I am seeing is that the hook I create infinitely re-renders because calling the new BasemapToggleVM() function mutates the properties it was supplied with and is therefore changing the input for my hook. For example, in my case I specifically see that the nextBasemap input property is being altered whenever I call new BaseMapToggleVM().

I am following a pattern similar to that which is described in this repo: https://github.com/rslibed/2023DS-build-a-custom-ui-for-api-widgets-bookmarks/tree/master/src/Components/Bookmarks

Reproduction: Link to codesandbox: https://codesandbox.io/p/sandbox/esri-infinite-hook-forked-nrkp39?file=%2Fsrc%2FBasemapToggleButton.tsx%3A10%2C6

Reproduction sample

https://codesandbox.io/p/sandbox/esri-infinite-hook-forked-nrkp39?file=%2Fsrc%2FBasemapToggleButton.tsx%3A10%2C6

Reproduction steps

  1. open sandbox
  2. check dev tools for infinite re-render.

Reproduction browser

chrome

Operating System (check https://whatsmyos.com)

Your OS is Windows 10* 64-bit

jonathandawe-geollect commented 11 months ago

I resolved the issue by passing in a new object instead of the props object directly. See the working code below:


import React from "react";
import Handles from "@arcgis/core/core/Handles";
import { watch } from "@arcgis/core/core/reactiveUtils";

interface HandlerSetup {
  basemapToggleVM: __esri.BasemapToggleViewModel;
  handles: Handles;
  onStateChange: (state: BasemapToggleVM["state"]) => void;
}
function addHandlers({
  basemapToggleVM,
  handles,
  onStateChange,
}: HandlerSetup) {
  handles.removeAll();

  handles.add([
    watch(
      () => basemapToggleVM.state,
      (state: BasemapToggleVM["state"]) => onStateChange?.call(null, state),
      { initial: true },
    ),
  ]);
}

export function useBaseMapToggleModel({
  nextBasemap,
  view,
}: __esri.BasemapToggleViewModelProperties) {
  const [basemapToggleVM, setBasemapToggleVm] =
    React.useState<__esri.BasemapToggleViewModel | null>(null);

  const [state, setState] =
    React.useState<BasemapToggleVM["state"]>("disabled");

  React.useEffect(() => {
    console.warn("re-rendered due to props input change");

    const basemapToggleModel = new BasemapToggleVM({ view, nextBasemap });
    setBasemapToggleVm(basemapToggleModel);

    return () => {
      basemapToggleModel.destroy();
    };
  }, [view, nextBasemap]);

  React.useEffect(() => {
    if (!basemapToggleVM) return () => {};

    const handles = new Handles();

    addHandlers({ basemapToggleVM, handles, onStateChange: setState });

    return () => {
      handles.removeAll();
      handles.destroy();
    };
  }, [basemapToggleVM]);

  return {
    state,
    toggle: () => {
      basemapToggleVM?.toggle();
    },
  };
}
`