mobxjs / serializr

Serialize and deserialize complex object graphs to and from JSON and Javascript classes
MIT License
766 stars 52 forks source link

missing data when using async deserializer and mobx observables #109

Open sven-petersen opened 5 years ago

sven-petersen commented 5 years ago

Hello,

I am facing issues with an async deserialisation in combination with mobx observables. I have put together a small example: https://jsfiddle.net/w5rzm9t8/ . I would have expected the output to be the same in both samples. Unfortunately in the second example I am missing some values after deserialisation.

There seems to be an issue when using async deserialisation when combined with an mobx Observable. For example it starts to work when I change

const thingResolver = (v, ctx, old, cb) => {
  setTimeout(() => cb(null, v.map(id => new Thing(id))), 0);
};

to

const thingResolver = (v, ctx, old, cb) => {
 cb(null, v.map(id => new Thing(id)));
}

Can you advise if it is an error on my side, not supported, or unexpected behavior? Thanks!

mweststrate commented 4 years ago

This looks like a bug indeed, would you be interested in submitting a PR with unit test?

On Tue, Nov 19, 2019 at 3:39 PM Sven Petersen notifications@github.com wrote:

Hello,

I am facing issues with an async deserialisation in combination with mobx observables. I have put a small example: https://jsfiddle.net/dp64g9xa/4/ . I would have expected the output to be the same in both samples. Unfortunately in the second example I am missing values after deserialisation.

There seems to be an issue when using async deserialisation on an mobx Observable. For example it starts to work when I change

const thingResolver = (v, ctx, old, cb) => { setTimeout(() => cb(null, v.map(id => new Thing(id))), 0); };

to

const thingResolver = (v, ctx, old, cb) => { cb(null, v.map(id => new Thing(id))); }

Can you advise if it is an error on my side, not supported, or unexpected behavior? Thanks!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mobxjs/serializr/issues/109?email_source=notifications&email_token=AAN4NBGY42MH4LPQLR5F32DQUQCC3A5CNFSM4JPFG5W2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4H2MHD7A, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN4NBBOGMOZLIEO4IZBJUDQUQCC3ANCNFSM4JPFG5WQ .

sven-petersen commented 4 years ago

Hi @mweststrate , I have created the tests. Hope this helps..

pyrogenic commented 4 years ago

See test case fixes https://github.com/aktivalux/serializr/pull/1 — let me know if the same change fixes your actual problem as well.

pyrogenic commented 4 years ago

@mweststrate it seems MobX relies on field declarations (see class Person from the docs) but Node 10 (which tavis uses for this project) doesn't support them:

/home/travis/build/mobxjs/serializr/test/mobx.js:15
519        data = undefined
520             ^
521
522  SyntaxError: Unexpected token =

I can reproduce this locally using nvm to run Node 10 instead of 13 (stable):

$ nvm exec 10 yarn run coverage
. . .
/Users/josh/github/pyrogenic/serializr/test/mobx.js:15
        data = undefined
             ^

SyntaxError: Unexpected token =
$ nvm exec 13 yarn run coverage
. . .
1..226
# tests 226
# pass  226

# ok