0

I'm working on building a file upload portal in React that allows for multiple concurrent uploads, using an amalgamation of some preexisting code and new code that I'm writing myself. The code base is pretty complex, so I will explain what's happening at a high level, illustrate the problem, and then provide a toy example to simulate what's happening in CodeSandbox.

The top level component has a files state variable from useState, which is an object that contains sub objects with information regarding each file that the user is currently uploading, that is being mapped across in the JSX and returning UI elements for each. Think:

  const uploadData = {
    1: {
      id: 1,
      name: "File 1",
      progress: 0
    },
    2: {
      id: 2,
      name: "File 2",
      progress: 0
    }
  };

  const [files, setFiles] = useState(uploadData);

  const progressElements = Object.values(files).map((file) => (
    <Progress key={file.id} value={file.progress} />
  ))

When an upload is initiated, existing code dictates that a callback is provided from the top level that receives an updated progress value for a given upload, and then sets that into state in files for the corresponding upload. This works perfectly fine when there is only one active upload, but as soon as a second file is added, the fact that the same files state object is being concurrently updated in multiple places at once bugs out the UI and causes the rendered JSX to be inaccurate. What is the correct way to handle concurrent updates to the same state at once?

Below is a super simplified (and hastily written, my apologies) sandbox as a toy example of what's going on. It's obviously not an exact replica of what's happening in the actual code, but it gets the general idea across. You can see that, with one upload going, the UI updates fine. But when additional uploads are added, any updates to the first overwrite the existence of the new upload in state and thus break the UI.

https://codesandbox.io/s/elastic-wozniak-yvbfo6?file=/src/App.js:133-297

const { useState, useEffect } = React;

const App = () => {
  const uploadData = {
    1: {
      id: 1,
      name: "File 1",
      progress: 0
    }
  };

  const [files, setFiles] = useState(uploadData);
  const [uploading, setUploading] = useState(false);

  const updateProgress = (uploadId, progress) => {
    setFiles({
      ...files,
      [uploadId]: {
        ...files[uploadId],
        progress
      }
    });
  };

  const filesArray = Object.values(files);

  const addNewFile = () => {
    const lastUpload = files[filesArray.length];
    const newId = lastUpload.id + 1;
    setFiles({
      ...files,
      [newId]: {
        id: newId,
        name: 'File ' + newId,
        progress: 0
      }
    });
  };

  return (
    <div className="App">
      {filesArray.map((file) => (
        <UploadStatus
          key={file.id}
          uploading={uploading}
          setUploading={setUploading}
          file={file}
          updateProgress={updateProgress}
        />
      ))}
      <button
        onClick={
          uploading ? () => setUploading(false) : () => setUploading(true)
        }
      >
        {uploading ? "Cancel Upload" : "Start Upload"}
      </button>
      {uploading && <button onClick={addNewFile}>Add New File</button>}
    </div>
  );
}

const UploadStatus = ({
  file,
  updateProgress,
  uploading,
  setUploading
}) => {
  useEffect(() => {
    if (!uploading) return;

    let calls = 0;

    const interval = setInterval(() => {
      calls++;
      updateProgress(file.id, calls * 10);
    }, 1000);

    if (calls === 10) {
      clearInterval(interval);
      setUploading(false);
    }

    return () => clearInterval(interval);
  }, [uploading]);

  return (
    <div key={file.id}>
      <p>{file.name}</p>
      <progress value={file.progress} max="100" />
    </div>
  );
}

ReactDOM.render(<App />, document.getElementById("root"));
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/18.0.0/umd/react.production.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react-dom/18.0.0/umd/react-dom.production.min.js"></script>
<body>
  <div id="root"></div>
</body>

Any help or thoughts would be greatly appreciated. Thanks!

Zach Kaigler
  • 279
  • 5
  • 15
  • 1
    Could you edit your question to a snippet instead of codesandbox, it works pretty much the same way. Click the `<>` icon and SO will open a snippet editor. – Keith Oct 05 '22 at 15:07
  • 1
    Use the callback form when calling `setFiles` so that the old closure isn't an issue and it'll work – CertainPerformance Oct 05 '22 at 15:36

1 Answers1

1

Based on what is inside your codesandbox - the issue is just with closures and missing items in dependency array of useEffect. I do understand why you want to have only the things you described in depsArray of useEffect - but if you dont provide all the things there - you will get an issue with closure and you will be calling the old function or refering old variable, what is happening in your codesandbox. Consider wrapping everything that is passed to the hooks or child components with useMemo, useCallback and other memoizing functions react provide and include everything that is needed in depsArray. There is a workaround with useRef hook to hold a reference to the specific function you want to call + useEffect with the only purpose to update this useRef variable.

So rough fix 1:

import { useEffect, useRef } from "react";

export default function UploadStatus({
  file,
  updateProgress,
  uploading,
  setUploading
}) {
  const updateProgressRef = useRef(updateProgress);
  const setUploadingRef = useRef(setUploading);

  useEffect(() => {
    updateProgressRef.current = updateProgress;
  }, [updateProgress]);

  useEffect(() => {
    setUploadingRef.current = setUploading;
  }, [setUploading]);

  useEffect(() => {
    if (!uploading) return;

    let calls = 0;

    const interval = setInterval(() => {
      calls++;
      updateProgressRef.current(file.id, calls * 10);
      if (calls === 10) {
        clearInterval(interval);
        setUploadingRef.current(false);
      }
    }, 1000);

    return () => clearInterval(interval);
  }, [file.id, uploading]);

  return (
    <div key={file.id}>
      <p>{file.name}</p>
      <progress value={file.progress} max="100" />
    </div>
  );
}

You will have to fix some logic here due to when any of the Uploadings is done - all the rest uploading will "stop" due to uploading and setUploading parameters.

What can simplify the fixing process is to modify addNewFile and updateProgress so they will not rely on closure captured files. setXXX functions from useState can recieve either the new value as a parameter, either a callback currentValue => newValue. So you can use callback one:

So rough fix 2:

const updateProgress = (uploadId, progress) => {
  setFiles((files) => ({
    ...files,
    [uploadId]: {
      ...files[uploadId],
      progress
    }
  }));
};

const addNewFile = () => {
  const lastUpload = files[filesArray.length];
  const newId = lastUpload.id + 1;
  setFiles((files) => ({
    ...files,
    [newId]: {
      id: newId,
      name: `File ${newId}`,
      progress: 0
    }
  }));
};

This fix will actually work better but still, wirings of states and components and depsArray should be fixed more precisiolly.

Hope that helps you to get the basic idea of what is going on and in which direction to dig with your issues.

Sergey Sosunov
  • 4,124
  • 2
  • 11
  • 15
  • Ah yes the setState callback is exactly what I needed - disappointed in myself for not coming to that conclusion sooner haha. Thank you! – Zach Kaigler Oct 05 '22 at 17:40