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

Safari 7, Observable Array issue #1374

Closed
Prichmondo opened this issue Mar 7, 2018 · 6 comments
Closed

Safari 7, Observable Array issue #1374

Prichmondo opened this issue Mar 7, 2018 · 6 comments

Comments

@Prichmondo
Copy link

On Safari 7, I having this error:
Error: [mobx.array] Index out of bounds

for an unknown reason, a Mobx Observable Array suddenly becomes a different Mobx object.
Debugging the object I noticed that, when I have the exception, the array.length property of the array is not a number but a function:
length () {
this.$mobx.atom.reportObserved();
return this.$mobx.dehanceValues(this.$mobx.values);
}

I have this issue only on Safari 7 with mobx 3

@mweststrate
Copy link
Member

mweststrate commented Mar 7, 2018

If you find a work around for this issue that doesn't impact the code too much feel free to submit a PR. But beyond that we won't be investigating investigation issues that occur on a 4 year old browser (there is most probably an issue in the Safari issue tracker that perfectly explain this behavior and is fixed in for example Safari 8).

@nykula
Copy link
Contributor

nykula commented Jul 7, 2018

Have more details and a reproduction. Safari is on a work phone where upgrade isn't possible. Running the following in console after loading MobX 4.3.1 onto an empty page:

window.alert = function (x) { document.body.innerText += "\n" + x; };

window.xs = mobx.observable.array();
mobx.autorun(function () { void window.xs.length; })
var successes = 0;

var interval = setInterval(function () {
  try {
    xs.push({});
  } catch (error) {
    alert(error);

    clearInterval(interval);

    alert("successes = " + successes);

    return;
  }

  successes++;
});

Number of successes is different every time but it always crashes after a few seconds. Complains about a RangeError, maximum stack size exceeded. Tracing it, see that at some point all properties defined on the observable array "shift" by a few positions, for example get() becomes replace(), get length() becomes what OP wrote etc. Internally called peek() after the shift is replaced by some method that attempts to call peek() itself, thus infinite recursion happens, leading to the crash.

If anyone has insight into what causes this, I'm interested in trying to make a workaround.

@nykula
Copy link
Contributor

nykula commented Jul 9, 2018

Breakage happens when ObservableArray keeps defining new properties on its prototype. Reserving more buffer items, like in #734, delays the issue a bit, for example 100000 instead of 1000 buys me up to a minute of time in the example above. Have to replace the value directly in MobX code, because "uptime" is smaller when configuring MobX from the app through the API like this:

mobx.configure({ arrayBuffer: 100000 });

Apparently, my real app is heavier, or another place breaks arrays in the same way. Still crashes.

@mweststrate
Copy link
Member

@makepost please note that commenting on closed issues is usually not noticed (like in this case, only after 3 months). Also not sure what is expected from MobX community here, as this is, as far as known, a bug in safari, not in MobX. But if increasing the arrayBuffer to an extreme value works around the issue that is good to know.

@nykula
Copy link
Contributor

nykula commented Oct 10, 2018

Wrote a note in case someone googles the error message and finds this issue, so that they have a less vague idea on what's happening. The bug is indeed on the native side, in WebKit JavaScriptCore JIT, and there's really nothing MobX could do about it. But since it affects no other major library (besides lodash, but its error looks different), a workaround is worth mentioning in the MobX issue tracker.

In the end I normalized my entities and used observable.ref with plain JS arrays, incidentally finding #1713 and sending a fix which you've since merged. MobX observable objects work fine inside these arrays, it's just the arrays that have to be ordinary.

After my post there was also one by @bsmith-cycorp, but he seems to have deleted it.

@mweststrate
Copy link
Member

Ah, probably that notification brought me here!

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

No branches or pull requests

3 participants