Skip to content
This repository has been archived by the owner on Oct 19, 2020. It is now read-only.

Remove .find usage #25

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

danmatthews
Copy link

I spent the better part of a day trying to debug an issue the plugin was causing on older android versions (~4.4.4 and below) - the Array.find polyfill works perfectly for what you use it for, but seems to trigger something internal to angular - perhaps it has it's own version of feature detection in which it uses an alternative to Array.find if it's not available in the environment.

The problem that arose was that the rest of my app code (i'm using Ionic framework, dump of my environment below) seemed to be throwing no end of exceptions (thrown by the polyfill itself) when the polyfill was included - removing it obviously fixed the exceptions, but broke the inapppurchase section of the app.

So i noticed you only seemed to be using .find in one small utility, so i swapped it out - i won't be offended if you don't want to merge this - i'll just keep using our forked version, but i thought i'd raise it anyway.

Thanks for the plugin, it's great, best i've found.

Removed the usage of `array.find` and the corresponding polyfill for backwards compatibility purposes.
@dougflip
Copy link

dougflip commented May 3, 2016

@danmatthews That stinks this was causing a hard to diagnose bug. I've definitely been there and it is frustrating!

I was surprised to see that mdn says Array.prototype.every has mobile support across the board. Using every feels like it might fit even better than find or a loop since it is designed to return a bool directly. What do you think?

@danmatthews
Copy link
Author

Sounds like a plan, i'll look into testing this on the offending phones later today.

@AlexDisler
Copy link
Owner

Thanks for looking into it, I'm definitely interested in fixing this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants