Skip to content

Conversation

@PiiXiieeS
Copy link

Here you are a pull request that fixes the concerts of Math.random() not guaranteed to be a high-quality RNG.
Anyway, I understand your remark and the fact that this may not be a serious issue for the use cases managed by this library, like framebus that we have started to use in our projects.

Credits for https://stackoverflow.com/questions/105034/how-to-create-a-guid-uuid

@PiiXiieeS PiiXiieeS closed this Apr 7, 2021
move from arrow function to standard function(c) to avoid issues with minifying with gulp task
@PiiXiieeS PiiXiieeS reopened this Apr 7, 2021
@PiiXiieeS
Copy link
Author

fix(uuid): remove arrow function
move from arrow function to standard function(c) to avoid issues with minifying with gulp task

@crookedneighbor
Copy link
Contributor

This is looking good. My one concern is for browser compatibility.

Uint8Array is not supported in IE9.

Crypto is not supported in IE9 or 10.

There are parts of the Braintree web SDK that still technically support IE9, so we'll need to wait until the next major version bump of the Braintree SDK to incorporate these changes. We're working on that now, so it shouldn't be too long. Until then, I'll leave this PR open.

Thanks so much for working on this! Hoping to get it merged in within the next couple of months.

@crookedneighbor
Copy link
Contributor

Also, using https://jsbench.me/, it shows it as being 97% slower to run it this way, though for Braintree's use case, I don't think that's a big deal. Running it a few times is still fast enough to not be noticeable by the end user.

@braintree/team-sdk would love to hear y'all's thoughts on this when you get a second.

@hacklschorsch
Copy link

@braintree/team-sdk would love to hear y'all's thoughts on this when you get a second.

I find it amusing that the SDK of a payment company uses Math.random() for randomness :) Hope nobody of those 340 k downloads per week on NPMJS associates the company name with serious business and uses your library to shoot a hole in their foot 🙏

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.

3 participants