Skip to content

fix(258): improve useEffect call#259

Open
spaudanjo wants to merge 1 commit into
JodusNodus:masterfrom
spaudanjo:258-improve-use-effect
Open

fix(258): improve useEffect call#259
spaudanjo wants to merge 1 commit into
JodusNodus:masterfrom
spaudanjo:258-improve-use-effect

Conversation

@spaudanjo

Copy link
Copy Markdown
  • add missing dependencies
  • add additional effect which stops the scanner when the component unmounts

Closes #258

HINT:
I only tested this roughly on my Chrome (Mac) and Chrome (Android) the new expected behaviour works. Not sure about potential breaking changes or performance drops (e.g. because of missing stop calls to previous versions of BrowserQRCodeReader).

* add missing dependencies
* add additional effect which stops the scanner when the component unmounts
@spaudanjo

Copy link
Copy Markdown
Author

Please let me know your thoughts on this @JodusNodus :)

@Corneliuus

Copy link
Copy Markdown

Can we have this merged into master please? @JodusNodus

@victorrseloy

Copy link
Copy Markdown

it would be pretty nice to have this merged @JodusNodus.

@Mistifiou

Mistifiou commented Aug 11, 2022

Copy link
Copy Markdown

Hi,

with this commit and this update on file I was able to fix #258 for my case.

      codeReader
        .decodeFromConstraints({ video }, videoId, (result, error, controls) => {
          controlsRef.current = controls
          if (isValidType(onResult, 'onResult', 'function')) {
            onResult(result, error, codeReader, controls);
          }
        })
        .catch((error: Error) => {
          if (isValidType(onResult, 'onResult', 'function')) {
            onResult(null, error, codeReader, controlsRef.current);
          }
        });
    }

@JayWelsh

Copy link
Copy Markdown

Hi @spaudanjo , I was wondering if you have any thoughts about your method of stopping the camera on mount vs the method used here?

#279

I've merged the linked PR into my fork ( https://github.com/JayWelsh/react-qr-reader ), and would like to merge more improvements into my fork (such as your dependency enhancement) which is why I am asking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve useEffect: include dependencies and introduce additional effect which stops scanner when component unmounts

5 participants