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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Temporarily mimic shift/unshift methods to workaround Safari/Webkit bug #122

Merged
merged 2 commits into from May 3, 2017

Conversation

jamsinclair
Copy link
Contributor

@jamsinclair jamsinclair commented May 2, 2017

As we know from https://bugs.webkit.org/show_bug.cgi?id=170264
After performing a shift operation on an array with large numbers, appending new values via bracket notation fails.

It seems to be in limbo when and if this is going to be resolved soon by Apple.

As we know this greatly affects bignumber.js in current iOS and Safari browsers.

Most notably affected is the infinite loop in the division function, #120.

Reading your comment #120 (comment)
I see you were concerned about hidden number accuracy problems due to shift and bracket notation being used elsewhere.

Proposed Solution

As a temporary workaround to help libraries downstream, could we mimic the shift operation with splice everywhere in this library?

This would ensure we don't encounter any unexpected results due to the webkit bug.

Splice seems to behave normally in buggy webkit and produces the same result as a shift.

var arr = [0, 2147483648]; 
arr.splice(0, 1);
arr[1] = 1;

// This will evaluate true in Safari and is expected
expect(arr).toEqual([2147483648, 1])

The test suites seem to pass ok locally 馃憤
Let's see what happens on CI 馃槃

Update

While the changes fixed the infinite loop, I noticed the browser test files when run locally on Safari failed.

This indicated other areas of this library had been affected and compromised by the WebKit bug.

I isolated this to the unshift() method. Substituting this for a slower alternative of Array.prototype.concat resulted in all 80335 tests passing.

IN TOTAL: 80335 of 80335 tests passed in 3.096 secs.

@jamsinclair jamsinclair changed the title Temporarily mimic shift method with splice(0, 1) to workaround Safari/Webkit bug Temporarily mimic shift/unshift methods to workaround Safari/Webkit bug May 3, 2017
@MikeMcl MikeMcl merged commit d7aa36f into MikeMcl:master May 3, 2017
@MikeMcl
Copy link
Owner

MikeMcl commented May 3, 2017

Looks good.
Thanks for your work on this.
I'll publish a new version presently.

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.

None yet

3 participants