Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sometimes camera feed get rendered twice in Nextjs reactStrictMode #2

Open
kaushalyap opened this issue Jan 20, 2023 · 12 comments
Open
Labels
html5-qrcode-bug Bugs that are unrelated to this library but presents in the original html5-qrcode-library.

Comments

@kaushalyap
Copy link

kaushalyap commented Jan 20, 2023

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",
@kaushalyap kaushalyap changed the title Sometimes camera feed get rendered twice Sometimes camera feed get rendered twice in Nextjs reactStrictMode Jan 20, 2023
@briosheje
Copy link
Owner

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.

@briosheje briosheje self-assigned this Jan 23, 2023
itsUndefined added a commit to itsUndefined/html5-qrcode that referenced this issue Jan 30, 2023
Fix race condition when render/clear is called in rapid succession. This issue was first spotted when trying to use this library in react:
Relevant issue : briosheje/react-html5-qrcode-reader#2
itsUndefined added a commit to itsUndefined/html5-qrcode that referenced this issue Jan 30, 2023
Fix race condition when render/clear is called in rapid succession. This issue was first spotted when trying to use this library in react:
Relevant issue : briosheje/react-html5-qrcode-reader#2
itsUndefined added a commit to itsUndefined/html5-qrcode that referenced this issue Jan 30, 2023
Fix race condition when render/clear is called in rapid succession. This issue was first spotted when trying to use this library in react:
Relevant issue : briosheje/react-html5-qrcode-reader#2
itsUndefined added a commit to itsUndefined/html5-qrcode that referenced this issue Jan 30, 2023
Fix race condition when render/clear is called in rapid succession. This issue was first spotted when trying to use this library in react:
Relevant issue : briosheje/react-html5-qrcode-reader#2
@itsUndefined
Copy link

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

@briosheje
Copy link
Owner

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
Copy link

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

@briosheje
Copy link
Owner

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

Thanks for investigating :)

@briosheje briosheje removed the triage label Feb 1, 2023
@briosheje briosheje removed their assignment Feb 1, 2023
@briosheje briosheje added the html5-qrcode-bug Bugs that are unrelated to this library but presents in the original html5-qrcode-library. label Feb 1, 2023
@kaushalyap
Copy link
Author

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
Copy link

itsUndefined commented Feb 2, 2023

@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
Copy link
Author

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

@itsUndefined
Copy link

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

itsUndefined added a commit to itsUndefined/html5-qrcode that referenced this issue Feb 11, 2023
Fix race condition when render/clear is called in rapid succession. This issue was first spotted when trying to use this library in react:
Relevant issue : briosheje/react-html5-qrcode-reader#2
@briosheje
Copy link
Owner

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

@kaushalyap
Copy link
Author

kaushalyap commented Jun 21, 2023

@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
Copy link
Author

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
html5-qrcode-bug Bugs that are unrelated to this library but presents in the original html5-qrcode-library.
Projects
None yet
Development

No branches or pull requests

3 participants