Mobx: TypeError: undefined is not a constructor (evaluating 'newItems.map(adm.makeChildReactive)')

Created on 14 Mar 2016  Â·  25Comments  Â·  Source: mobxjs/mobx

I can't figure out this error, but it seems like it may be a problem with mobx. But it might not.

The top of the stack trace has spliceWithArray and replace (from observablearray.ts), and then my code, which is calling collection.push(myobservableobject) where collection is an empty observable array.

One point of confusion is why is my code calling push but then the stack trace shows that I called to replace?

I will see if I can put together a minimal case to reproduce.

Most helpful comment

We're seeing similar flaky test results in PhantomJS 2.1.13, node 6.7.0, karma 1.3.0, mobx 2.5.1

Every so often an array marked as @observable isn't actually registering as an ObservableArray. It also only happens when bundled with our full spec suite. Compiling and running the spec by itself doesn't produce the error. The error also never occurs on Chrome or Electron. But we need something headless to run in CI. I guess I need to figure out how to use karma with xvfb. :(

All 25 comments

can't figure out how to reproduce this reliably. However, I found a workaround: use chrome for my automated tests instead of PhantomJS. It seems to be something that only happens under PhantomJS.

Closing since I am unable to provide reproducible case.

Ok, tnx for sorting out!

This happens to me on iOS 7: "Mozilla/5.0 (iPhone; CPU iPhone OS 7_1_2 like Mac OS X) AppleWebKit/537.51.2 (KHTML, like Gecko) Version/7.0 Mobile/11D257 Safari/9537.53".

The error is a bit different, but I believe it's the same issue, as the situation is the same -- empty observable array, and pushing an object literal to that array:

TypeError: 'undefined' is not a function (evaluating 'newItems.map(adm.makeChildReactive)')

Still trying to pin-point the culprit though.

Looking in to it a bit more. For some strange reason on iOS7 Safari (the older iPhone I mentioned above), when I have an observable array, e.g.:
@observable items = [];

Then elsewhere in the componentWillMount method (doesn't really matter if item is a simple or observable object):
this.items.push(item);

The stack moves to ObservableArray.prototype.replace = function (newItems) {, not ObservableArray.prototype.push as it does on new browsers.

From there on the stack breaks, obviously, as newItems is not an array but the passed in object.
Any ideas?

Was able to make it work by making the order of non-enumerable properties of the object to correspond to the order of the prototype methods:

makeNonEnumerable(ObservableArray.prototype, [
    "constructor",
    "observe",
    "clear",
    "replace",
    "toJSON",
    "peek",
    "find",
    "splice",
    "push",
    "pop",
    "shift",
    "unshift",
    "reverse",
    "sort",
    "toString",
    "toLocaleString",
    "remove",
    "split"
]);

Is that something that old safari might be doing with the prototype? By the looks of it the methods are getting mapped incorrectly.

And, by the way, @mweststrate, there's no split method defined in the prototype either way, so it's undefined, but that is not really relevant to the issue described above (just saying).

@bstst thanks for diving into this, I'll at least apply the changes you did above and remove the split (really wondering how it got there).

Probably it isn't the issue here, but know that sourcemaps might play dirty tricks with you if it appears that you are in a function you didn't expect to end up in. if you are unsure, disable / strip them.

Updated and released as 2.1.2, does this solve the issue? (which would be really weird imho but I guess it is worth trying ;-))

Yes, the issue is not reproducible anymore for me with 2.1.2. Maybe @jeffgran might want to give it a go with PhantomJS.

As for the fix itself -- I'm still trying to wrap my head around it ;))

@bstst only thing that I can imagine is that the non-enumerable("splice") caused a silent exception, which wasn't in the way anymore after putting it at the end at the list. But that still doesn't explain the wrong stack frames...

@mweststrate, nope, I did try removing "split" from the old array before reordering it, and the methods were still being mixed up.

I remember I was once taught not to trust sorting of arrays and object properties. Now I don't know what to trust at all ;))

Haha yeah this is extremely scare ain't it? I close it for now, if similar stuff happens in the future for PhantomJS it can be reopened.

I'm having a similar issue as mentioned in this thread and starting to troubleshoot it, but the thing that's confusing to me is that @bstst and @mweststrate, you are mentioning a phantomjs version 2.1.2. But it doesn't appear to me that 2.1.2 has been released yet: https://bitbucket.org/ariya/phantomjs/downloads. Am I missing something?

@superplussed, try https://github.com/Medium/phantomjs/releases (or see npm info phantomjs versions)

I was seeing similar things in PhantomJS (and we had a weird Safari bug as well) where in my tests replace was becoming clear. Perhaps to add to the discussion, at first replace is replace (console.log replace.toString), but in the execution of a single test at some point it becomes reassigned and the signature matches that of clear.

Edit: this is with PhantomJS 2.1.1 and MobX 2.3.3. I know our app wasn't working in Safari a few weeks ago as well, haven't checked lately.

@javascriptjedi @mweststrate We are running into a very similar issue with PhantomJS 2.1.1 and MobX 2.4.2. For us the problem is with push. as @javascriptjedi mentioned console.log(_theObservableArray_.push.toString); is correct (like immediately after declaring) then when the it block hits in the test it is now referencing find for some reason.

Just some things to note as I know "for some reason" isn't super helpful:

  • This was only occurring on a component that had multiple children i.e.
      <CheckboxInput {...props} >
        <CheckboxOption value='green'>Green</CheckboxOption>
        <CheckboxOption value='red' label='Red'/>
        <CheckboxOption value='blue'/>
      </CheckboxInput>
  • We are using a util function that checks if the value being checked is already in the observable array. To accomplish this is uses indexOf. When indexOf is removed the problem is not present.

@mikedklein could you set up a minimal reproduction? That would be really useful as these problems are reported now and then, but I didn't see a reliable setup for it so far

@mweststrate definitely will try to get a setup over the weekend for you.

EDIT: This example was way too complicated @krtools example is more easy to follow and reliable.

@mweststrate so I got something together but I am really struggling to get this to happen reliably. Once I cleaned the project out it seemed like the error stopped. So I nested a timeout inside of a loop in the test and have gotten the error to happen sometimes. I have to run it multiple times for the error to pop. It almost seems like this is a race condition. If you mess with the loop in the test while the tests are running you should be able to reproduce it. The more I dug into this the more it seems like bug in phantom and not much to do with MobX but I can't be sure because frankly I have no clue what is happening here.

You will know right away when you hit it, the stack trace will start with
undefined is not a constructor (evaluating 'predicate.call(thisArg, items[i], i, this)')
which is part of the find method in MobX instead of push.

I hope this helps I wish I could've got to this come up more reliably.

Setup of the repo should be ready to go:

npm install

to run the actual form
npm start

to run the tests
npm run test:watch

Also my apologies for any extraneous stuff in the repo was just trying to get a quick and dirty way of making this happen.

@mikedklein @mweststrate Here's a boiled down version that i was able to reliably reproduce on my machine (mobx 2.4.2 and phantomjs 2.1.12 on windows)

https://gist.github.com/krtools/819293023786a240de144e2a970562e6

can someone verify this happens in Safari as well or is it a PhantomJS only issue? (In the latter case I probably won't fix it, PhantomJS isn't an officially supported platform atm, just use electron (or submit a PR ;-))

@mweststrate tested in safari with same test setup as presented by @krtools and no errors.

We're seeing similar flaky test results in PhantomJS 2.1.13, node 6.7.0, karma 1.3.0, mobx 2.5.1

Every so often an array marked as @observable isn't actually registering as an ObservableArray. It also only happens when bundled with our full spec suite. Compiling and running the spec by itself doesn't produce the error. The error also never occurs on Chrome or Electron. But we need something headless to run in CI. I guess I need to figure out how to use karma with xvfb. :(

yeah xfvb + electron is great. phantom in our experience causes more bugs
then it catches

Op wo 16 nov. 2016 22:32 schreef Damon Black [email protected]:

We're seeing similar flaky test results in PhantomJS 2.1.13, node 6.7.0,
karma 1.3.0

Every so often an array marked as @observable
https://github.com/observable isn't actually registering as an
ObservableArray. It also only happens when bundled with our full spec
suite. Compiling and running the spec by itself doesn't produce the error.
The error also never occurs on Chrome or Electron. But we need something
headless to run in CI. I guess I need to figure out how to use karma with
xvfb. :(

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/160#issuecomment-261078826, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABvGhAEi9WbFnviJGYjXnY7kdXQZqdt3ks5q-3Z2gaJpZM4HwQRb
.

Closing this issue for now, until there is a reliable reproduction on a modern well supported browser.

Was this page helpful?
0 / 5 - 0 ratings