briosheje / react-html5-qrcode-reader

A wrapper around html5-qrcode for react and SSR
Apache License 2.0
13 stars 7 forks source link

Sometimes camera feed get rendered twice in Nextjs reactStrictMode #2

Open kaushalyap opened 1 year ago

kaushalyap commented 1 year ago

First of all thanks for library and making it React 18 compatible, it is hard to find a maintained QR reader library these days. But I am experiencing a bug with your library, which is described below.

At first I was using https://github.com/scanapp-org/html5-qrcode-react which make a wrapper around html5-qrcode library, in that also sometimes camera video feed get rendered twice(one below the other, with two stop scanning buttons), I thought some mis handled case in his code, so I tried your library hoping this case may be handle by your library but even in your library camera feed get rendered twice. This only seem to happen in development mode of Nextjs when reactStrictMode: true,.

I am using following package versions

"next": "^12.3.4",
"react": "18.2.0",
"react-dom": "18.2.0",
briosheje commented 1 year ago

Can you please provide a stackblitz or similar to replicate the issue?

Also, remember that strict mode always performs two re-renders, this is by design in React.

itsUndefined commented 1 year ago

I just opened a PR on the main project to fix this issue

briosheje commented 1 year ago

I just opened a PR on the main project to fix this issue

Ok, so I suppose the original issue is directly relying on html5-qrcode directly, so this library does not need to be updated because it relies on the minified/compiled html5-qrcode library.

itsUndefined commented 1 year ago

Right. It's an issue with html5-qrcode directly. You can follow the PR if you want: https://github.com/mebjas/html5-qrcode/pull/686

briosheje commented 1 year ago

@itsUndefined I will keep the issue opened here until it gets closed in the html5-qrcode repo.

Thanks for investigating :)

kaushalyap commented 1 year ago

Right. It's an issue with html5-qrcode directly. You can follow the PR if you want: mebjas/html5-qrcode#686

I thought some mistake in this library due useEffect being run on twice in strict mode (missing clean up)

itsUndefined commented 1 year ago

@kaushalyap you are right. You should also call the clear() method of the Html5QrcodeScanner instance inside the useEffect "destructor" but if my fix doesn't get merged that would not fix the problem. That's why you couldn't get it to work even if you were using https://github.com/scanapp-org/html5-qrcode-react.

Example code:

useEffect(() => {
    let html5QrcodeScanner 
    if (Html5QrcodeScanner) {
      // Creates anew instance of `HtmlQrcodeScanner` and renders the block.
      html5QrcodeScanner = new Html5QrcodeScanner(
        "reader",
        { fps: 10, qrbox: {width: 250, height: 250} },
        /* verbose= */ false);
      html5QrcodeScanner.render(
        (data: any) => console.log('success ->', data), 
        (err: any) => console.log('err ->', err)
      );
    }

    return () => {
        html5QrcodeScanner?.clear()
    }
  }, [Html5QrcodeScanner]);
kaushalyap commented 1 year ago

@itsUndefined clean up does not fix the issue for me.

itsUndefined commented 1 year ago

That's because the patch that I sent to the html5-qrcode package is not yet accepted

briosheje commented 1 year ago

@itsUndefined Do you know whether this was merged / fixed in the original lib?

kaushalyap commented 1 year ago

@briosheje I am experiencing camera not being turned off even after page change both in prod and dev, even with following

 return () => {
      html5QrcodeScanner = null
    }
  }, [Html5QrcodeScanner])

It seems something are not getting teared down properly so probably there is a memory leak.

kaushalyap commented 1 year ago

@itsUndefined I tried your build source from you PR, I am getting a typed error

TypeError: Cannot read properties of undefined (reading 'Html5QrcodeScanner')