trussworks / react-uswds

USWDS 3.0 components built in React
https://trussworks.github.io/react-uswds/
Apache License 2.0
181 stars 81 forks source link

[fix] FilePreview component breaks FileReader use in onChange events #2433

Open patsier-cms opened 1 year ago

patsier-cms commented 1 year ago

ReactUSWDS Version & USWDS Version:

@trussworks/react-uswds: 4.2.1 @uswds/uswds: 3.1.0

Describe the bug

When a function that uses FileReader is called from an onChange event in the FileInput component, it throws an error because FileReader is already being used on any added files in the FilePreview component.

Ideally, users should be able to disable calling FileReader inside the FilePreview component so that it can still be used in change events.

To Reproduce Steps to reproduce the behavior:

  1. On Chrome, create a component using FileInput with an onChange event using FileReader
  2. Add a file
  3. See error Uncaught DOMException: Failed to execute 'readAsDataURL' on 'FileReader': The object is already busy reading Blobs.

Expected behavior

Using FileReader within the FileInput component should be optional

Additional context

My specific use case for this is to be able to validate text file input in onChange events, and I'd be happy to put in a PR that allows for disabling calling the FileReader which seems to only be used for generating image previews. Thanks for taking a look!

https://github.com/trussworks/react-uswds/blob/c31423439cf6a30117fd327ee6cc2b1aacdb9124/src/components/forms/FileInput/FilePreview.tsx#L23-L34

Device and Browser Information (please complete the following information if describing a UI bug):

allthesignals commented 8 months ago

This happens to me when I'm running Next and React 18, which this package isn't currently supporting...

While most of the components work fine, there's something going on with FileInput component where it's being called twice. When I "correctly" align versions (Next with React 17 with this package), the duplicate instantiation behavior goes away...

Note that regardless of versioning, the problem doesn't occur in the build output, only in the dev environment (next dev).

TomNUSDS commented 6 months ago

I just hit this bug and was going to report it.

It's as easy to repro as:

  1. npm create vite@latest (choose React, typescript)
  2. cd [project]
  3. npm i @trussworks/react-uswds
  4. Replace App.tsx page content with <FileInput name={"test"} id={'test'} />.
  5. Run via dev command in package.json and select any image file. Result: page is rendered blank and the console has a bunch of error messages Uncaught DOMException: Failed to execute 'readAsDataURL' on 'FileReader': The object is already busy reading Blobs

Workaround:

  1. Open main.tsx
  2. Remove the <React.StrictMode> around <App/>
  3. Rerun and select any image file Result: Page works as expected.

The problem seems to be that that React.StrictMode (ref) causes a quick double rendering of the <FilePreview> component.

A workaround is to remove the <React.StrictMode> wrapper around <App />.

One fix to explore is to just prevent double loading in FilePreview. An interesting approach can be found here: https://www.ericbroucek.us/blog/useeffect-only-once-vs-after-initial-render/ but there are others. You'll need to test and make sure reselecting a different image reloads the preview.

Another possible fix is to allow disabling of the FilePreview or allow user to provide their own previewer component in the FileInput props. But the solution still has the issue that folks will run into the bug and somehow need to know they should disable/replace the FilePreview.

TomNUSDS commented 6 months ago

Another thought: FilePreview aspect of the FileInput component is cool, but there doesn't seem to be any way to set a defaultValue for the preview?

Any form that allows a user to replace an existing image, should show the current image before the user selects an upload file to replace it. So, maybe some way to override the FilePreview subcomponent in the FileInput is a way to control this nuance. Or, at a minimum allow the Preview to be disabled.

Steven7926 commented 4 months ago

I have also just run into this issue, The workaround was to just remove strict mode from the app. "@trussworks/react-uswds": "^9.0.0" "@uswds/uswds": "3.7.1" Vite React application on Chrome running on Windows.

TomNUSDS commented 4 months ago

Yeah, to fix this component would be something like this?

  const [showGenericPreview, setShowGenericPreview] = useState(false)
  const firstRenderRef = useRef(false)

  useEffect(() => {
    if (firstRenderRef.current) {
        // already run, do nothing
        return
    }
    // only run once
    firstRenderRef.current = true

    fileReaderRef.current.onloadend = (): void => {
      setIsLoading(false)
      setPreviewSrc(fileReaderRef.current.result as string)
      fileReaderRef.current.onloadend = null // is only run once
    }

    fileReaderRef.current.readAsDataURL(file)
  }, [])