ericblade / quagga2

An advanced barcode-scanner written in Javascript and TypeScript - Continuation from https://github.com/serratus/quaggajs
MIT License
758 stars 85 forks source link

Issue when including Quagga in Angular application (Pollyfilling Promise) #130

Closed WardLootens closed 4 years ago

WardLootens commented 4 years ago

As soon as I refer to Quagga in an Angular application, the application stops working. The following error occurs:

Zone.js has detected that ZoneAwarePromise `(window|global).Promise` has been overwritten.
Most likely cause is that a Promise polyfill has been loaded after Zone.js (Polyfilling Promise api is not necessary when zone.js is loaded. If you must load one, do so before loading zone.js.)

So I guess Quagga also provide some polyfills (ao for Promise). I cannot entirely figure out where and how the polyfills are added, and if this can be disabled in some way....

ericblade commented 4 years ago

Interesting. Promise shouldn't need to be polyfilled at any point, i'm pretty sure that no browser implemented getUserMedia or file access prior to Promise :-D

This also sounds like some sort of an effect of the upgrades to webpack, and if you go back to 0.0.10 or so, it probably works. What an incredibly complex system. :-S

I've never used Angular -- would it be possible you could make an absolute minimal example that triggers it?

... I have to seriously question the sanity of a software in Javascript that requires a super-implementation of something built-in to work ... but that's a slightly different topic.

ericblade commented 4 years ago

I'm having a suspicion that changing the configuration of @babel/preset-env to "useBuiltIns": "entry" might solve this issue, as it decreases bundle sizes drastically, and the tests still work, which is why I added @babel/preset-env to begin with, was to get the tests to work, since various functions needed to be polyfilled to even get PhantomJS off the ground. I'm attempting to run my external tests with that change now, to see if that breaks anything else anywhere... since we now know after several point releases, that the test suite included with q2 is not anywhere near comprehensive enough.

ericblade commented 4 years ago

hey @WardLootens any chance you could give it a try with this pull req added? https://github.com/ericblade/quagga2/pull/131

... i'm also having visions of starting to build a new test suite, that if i manage to engineer it correctly, we could potentially add in tests that use Angular and React and validate that things work with those.. it's a bit overkill for what should be necessary, but this whole lib is quite complex.

WardLootens commented 4 years ago

Yes, of course, I am eager to test this, but currently struggling to get this version from pull request working I'm trying with npm install --save ericblade/quagga2#pull/131/head, but the folder in node_modules does not seem complete then, causing an error (Can't resolve '@ericblade/quagga2') when importing Quagga...

I will continue to try to get this working...

WardLootens commented 4 years ago

Ok, it was just a matter of RTFM...

Seems to work now!

Thanks!

ericblade commented 4 years ago

... it works with Angular now? my guess is that it's now only pulling in the polyfills when it runs the test suite.. which is i think sort of what's desired.. since phantomjs is really the only particularly old thing we need to support.. at least, if i understand this configuration correctly, it should only be pulling in polyfills referenced via import in the entry files, and the entry for the main lib doesn't reference any of the core-es polyfills, only the entry for tests does..

i think. webpack is still quite a complex mystery to me.

WardLootens commented 4 years ago

... it works with Angular now? Yes it does!

I'm not sure exactly how the entry value behaves, but as you said, I don't think there won't be much polyfills needed for browsers that do already support getUserMedia :-D

i think. webpack is still quite a complex mystery to me. Well, your not alone ;-) But I'm learning a lot these days...

Anyway, PR #131 may be merged I think

ericblade commented 4 years ago

good deal. as far as i can tell it didn't break any other cases ..