0

I got a react functional component:

const DataGrid = (props) =>
{          
    const [containerName, setContainerName] = useState("");                                                                   
    const [frameworkComponents, setFrameworkComponents] = useState(
      {customLoadingOverlay: LoadingOverlayTemplate,
      customNoRowsOverlay: UxDataGridCustomNoRows,
      editButton: params => <ViewAndDeleteSetting {...params}  
                                                  openAddConfigurationsWindow={openAddConfigurationsWindow}
                                                  onDeleteSetting={onDeleteSetting}/>,
     });

useEffect(async () =>
    {
      if(props.containerName && props.containerName !== "")
      {
        setContainerName(props.containerName);
      }
    },[props.containerName]);
.
.
.
const onDeleteSetting = async (settingKey) =>
{
  console.log("ON DELETE AND CONTAINER NAME:");
  console.log(containerName); //HERE THE CONTAINER NAME IS EMPTY
   ...
}
return (
  <UxDataGrid 
            frameworkComponents={frameworkComponents}/>
);

The container name inside useEffect exists and is not empty. As you can see in the comment in onDeleteSetting, the containerName is empty when this callback is invoked. I tried adding this to the useEffect after setContainerName:

setFrameworkComponents({customLoadingOverlay: LoadingOverlayTemplate,
        customNoRowsOverlay: UxDataGridCustomNoRows,
        editButton: params => <ViewAndDeleteSetting {...params}  
                                                         openAddConfigurationsWindow={openAddConfigurationsWindow}
                                                         onDeleteSetting={onDeleteSetting}/>,
            });

That didn't work.

How can I get the name inside the callback? There is no special need to leave that frameworkComponents struct in the state.. it can also be moved to somewhere else if you think its better

CodeMonkey
  • 11,196
  • 30
  • 112
  • 203
  • The `containerName` is empty in the `onDeleteSetting` handler because your `frameworkComponents` variable is only set initially. Even if you try to set it after calling `setContainerName`, React's `useState` update [isn't immediate](https://linguinecode.com/post/why-react-setstate-usestate-does-not-update-immediately). If you really want to do something like that, you can add another `useEffect` with `containerName` as the dependency and call `setFrameworkComponents` in it. But your whole code structure looks like bad practice, try reafactoring to avoid setting whole components in your `state` – Kapobajza Jan 12 '21 at 13:45
  • @Kapobajza could you add an answer with the way you suggest to refactor? – CodeMonkey Jan 12 '21 at 14:44
  • Issue related to stale closure, https://stackoverflow.com/questions/63307310/how-can-i-access-react-state-in-my-eventhandler/63308242#63308242 , this might help you. – Siddharth Pachori Jan 12 '21 at 16:05

3 Answers3

0

You almost certainly shouldn't be storing it in state. Props are essentially state controlled by the parent. Just use it from props. Copying props to state is usually not best practice.

If you're looking at one of the very rare situations where it makes sense to set derived state based on props, this page in the documentation tells you how to do that with hooks. Basically, you don't use useEffect, you do your state update right away.

Here's a full quote from the linked documentation:

How do I implement getDerivedStateFromProps?

While you probably don’t need it, in rare cases that you do (such as implementing a <Transition> component), you can update the state right during rendering. React will re-run the component with updated state immediately after exiting the first render so it wouldn’t be expensive.

Here, we store the previous value of the row prop in a state variable so that we can compare:

function ScrollView({row}) {
  const [isScrollingDown, setIsScrollingDown] = useState(false);
  const [prevRow, setPrevRow] = useState(null);

  if (row !== prevRow) {
    // Row changed since last render. Update isScrollingDown.
    setIsScrollingDown(prevRow !== null && row > prevRow);
    setPrevRow(row);
  }

  return `Scrolling down: ${isScrollingDown}`;
}

This might look strange at first, but an update during rendering is exactly what getDerivedStateFromProps has always been like conceptually.

If you did it the same way they did in that example, your component would still render with containerName set to the default state (""), it's just that it will then almost immediately re-render with the updated containerName. That makes sense for their example of a transition, but you could avoid that by making the prop's initial value the state's initial value, like this:

const DataGrid = (props) => {
    const [containerName, setContainerName] = useState(props.containerName); // *** ONLY USES THE INITIAL PROP VALUE
    const [frameworkComponents, setFrameworkComponents] = useState(
        // ...
     });

    // *** Updates the state value (on the next render) if the prop changes
    if (containerName !== props.containerName) {
        setContainerName(props.containerName);
    }

    // ...
};

Every time the containerName prop changes, though, your component will render twice, which brings us back full circle to: Don't store it in state, just use it from props. :-)


Stepping back and looking at the component as a whole, I don't think you need any state information at all, but if your goal is to avoid having the frameworkComponents you pass UxDataGrid change unnecessarily, you probably want useMemo or React.memo rather than state.

For instance, with useMemo (but keep reading):

const DataGrid = ({containerName}) => {
    const frameworkComponents = useMemo(() => {
        const onDeleteSetting = async (settingKey) => {
            console.log("ON DELETE AND CONTAINER NAME:");
            console.log(containerName);
            // ...
        };
        return {
            customLoadingOverlay: LoadingOverlayTemplate,
            editButton: params => <ViewAndDeleteSetting {...params}  
                openAddConfigurationsWindow={openAddConfigurationsWindow}
                onDeleteSetting={onDeleteSetting} />,
        };
    }, [containerName]);

    return (
        <UxDataGrid frameworkComponents={frameworkComponents} />
    );
};

But if componentName is your only prop, it may well be even simpler with React.memo:

const DataGrid = React.memo(({containerName}) => {
    const onDeleteSetting = async (settingKey) => {
        console.log("ON DELETE AND CONTAINER NAME:");
        console.log(containerName);
        // ...
    };
    return (
        <UxDataGrid frameworkComponents={{
            customLoadingOverlay: LoadingOverlayTemplate,
            editButton: params => <ViewAndDeleteSetting {...params}  
                openAddConfigurationsWindow={openAddConfigurationsWindow}
                onDeleteSetting={onDeleteSetting} />,
        }} />
    );
});

React.memo memoizes your component, so that your component function is only ever called again when the props change. Since everything in the component needs to update based on the componentName prop changing, that looks like a good match (but I don't know what UxDataGrid is).

T.J. Crowder
  • 1,031,962
  • 187
  • 1,923
  • 1,875
  • I tried with the useMemo but I still got an empty string – CodeMonkey Jan 12 '21 at 14:41
  • @YonatanNir - If so, then either you haven't implemented it correctly, or `UxDataView` isn't using the latest `onDeleteSetting`. – T.J. Crowder Jan 12 '21 at 14:56
  • useMemo itself seem to be working since I put console log there and I see it first renders with an empty string but when i trigger the rendering with the new props, it does call the useMemo again. Just for some reason the callback inside still doesn't use the latest data – CodeMonkey Jan 12 '21 at 15:04
  • i even tried to define the struct without the useMemo so it would just recreate each time, and still it doesn't work – CodeMonkey Jan 12 '21 at 15:37
  • @YonatanNir - It sounds like a problem with `UxDataView` (or this way of using it). In particular, I note that one of the properties is a function that creates the delete button. It sounds like that may be getting used, and then not reused when the component is rerendered. – T.J. Crowder Jan 12 '21 at 15:45
0

Try this in your useEffect, update the onDeleteSetting function with the new containerName when it's updated

.....
useEffect(async() => {
  if (props.containerName && props.containerName !== "") {
    setContainerName(props.containerName);
    
    // move this function here
    const onDeleteSetting = async(settingKey) => {
      console.log("ON DELETE AND CONTAINER NAME:");
      // use props.containerName since the state update is async
      console.log(props.containerName);
      ...
    }

    // update your components with the updated functions
    setFrameworkComponents(prevComponents => ({
      ...prevComponents,
      editButton: params => 
              <ViewAndDeleteSetting
                {...params}                                                  
                openAddConfigurationsWindow={openAddConfigurationsWindow}
                onDeleteSetting={onDeleteSetting}
              />,
    }));
  }
}, [props.containerName]);
.....

This should provide the updated state with the updated function, if it works, I can add more details.

Sudhanshu Kumar
  • 1,926
  • 1
  • 9
  • 21
0

The problem was with how I tried passing props to ViewAndDeleteSetting. If you want to pass prop to a cell rendered component, you shouldn't be doing it in frameworkComponents, but rather you need to do it in the column definition like this:

useEffect(() =>
{
  let columns = [{headerName: '', cellRenderer: 'editButton', width: 90, editable: false, 
                  cellRendererParams: {
                    openAddConfigurationsWindow: openAddConfigurationsWindow,
                    onDeleteSetting: onDeleteSetting
                }},
                .. other columns
                ]

  setColumnDefinition(columns);
},[props.containerName]);

The columns with the cellRendererParams do gets recreated in the useEffect when the name changes, and then the component can access this params regularly via its props

CodeMonkey
  • 11,196
  • 30
  • 112
  • 203