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

feat: only install polyfill if there is no native support #447

Merged
merged 2 commits into from
Sep 2, 2024

Conversation

pastc
Copy link

@pastc pastc commented Sep 2, 2024

By default the BarcodeDetector polyfill was always install on all platforms. That came at a performance cost in raw scanning speed on supporting platforms that already had native support for the Barcode Detection API.

Since the Barcode Detection API is fully implemented on safari, there is no point on using the polyfill everywhere.

@gruhn
Copy link
Owner

gruhn commented Sep 2, 2024

Sorry, now that I think about it, this might break applications with SSR (Server Side Rending). I think the BarcodeDetector polyfill would get applied during SSR but at this point globalThis is not window so the polyfill is never available at runtime. Maybe it's better to use something like:

function newBarcodeDetector(...args) {
  if ('BarcodeDetector' in window) {
    return new window.BarcodeDetector(...ags)
  } else {
    return new BarcodeDetector(...args)
  }
}

PS: that permission issue with the Github action is annoying. I'll see how to work around that.

@gruhn gruhn changed the base branch from master to detector-polyfill-on-demand September 2, 2024 11:12
@gruhn gruhn merged commit 2bd4763 into gruhn:detector-polyfill-on-demand Sep 2, 2024
1 check failed
@gruhn
Copy link
Owner

gruhn commented Sep 2, 2024

Nevermind, I don't know how to preview deploy this fork branch. I'm merged into a local branch and I'll take care of SSR.

@pastc
Copy link
Author

pastc commented Sep 2, 2024

Thanks!

gruhn added a commit that referenced this pull request Sep 2, 2024
gruhn added a commit that referenced this pull request Sep 2, 2024
gruhn added a commit that referenced this pull request Sep 6, 2024
Still using polyfill for `QrcodeDropZone` and `QrcodeCapture` even on
platforms with native `BarcodeDetector` is available, because on some
of them `BarcodeDetector.detect` does not support `Blob` / `File`
inputs.

See: #447
gruhn added a commit that referenced this pull request Sep 6, 2024
Still using polyfill for `QrcodeDropZone` and `QrcodeCapture` even on
platforms with native `BarcodeDetector` is available, because on some
of them `BarcodeDetector.detect` does not support `Blob` / `File`
inputs.

See: #447
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.

2 participants