toJS() behaves unexpected

[stnwk]

Hi :)

I came across an unexpected behavior from toJS().

It states in the docs:

toJS():
Recursively converts an (observable) object to a javascript structure.

Example 1 (works as expected)

const someIds = observable.array([10, 20, 30]);
const someJSIds = toJS(someIds)

-> someJSIds is a plain js array

But

Example 2 (works unexpected)

const someIds = observable.array([10, 20, 30]);
const options = toJS({ someIds }); // notice i'm passing an object here

-> options.someJSIds is now still an observable array

Taken from the documentation I reckon this is a bug / undesired behavior: it should take (observable) data structure and return a plain js object. Just because the top-level object is not an observable structure, doesn't mean toJS() should not convert possible nested values.

Using mobx 4.3.0

[mweststrate]

This is working as expected; as soon as toJS runs into something that is not observable, it won't recurse any further (just to avoid bumping in the window object for example and cloning that thing entirely ;-)). Since you are not giving an observable to toJS in your second example, toJS is basically a no-op Op do 24 mei 2018 om 15:30 schreef Stefan <notifications@github.com>:

…

Hi :) I came across an unexpected behavior from toJS(). It states in the docs: toJS(): Recursively converts an (observable) object to a javascript structure. Example 1 (works as expected) const someIds = observable.array([10, 20, 30]);const someJSIds = toJS(someIds) -> someJSIds is a plain js array But Example 2 (works unexpected) const someIds = observable.array([10, 20, 30]);const options = toJS({ someIds }); // notice i'm passing an object here -> options.someJSIds is now still an observable array Taken from the documentation I reckon this is a bug / undesired behavior: it should take (observable) data structure and return a plain js object. Just because the top-level object is not an observable structure, doesn't mean toJS() should not convert possible nested values. Using mobx 4.3.0 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1557>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABvGhIUbM_OV7JMgl_P0dIytLWPMJsaFks5t1rYRgaJpZM4UMQDw> .

[stnwk]

Ok, thanks for your quick response!

Either way is fine.
But in this case the documentation is misleading as it clearly states "Recursively converts an (observable) object to a javascript structure." - since "(observable)" is writting in parenthesis it gives the impression it would also recursively convert a non-observable object to a javascript structure in my second example.

Do you see what I mean?

[mweststrate]

Yes, PR is welcome 😊(there is an edit button on top of the page) Op do 24 mei 2018 16:09 schreef Stefan <notifications@github.com>:

…

Ok, thanks for your quick response! Either way is fine. But in this case the documentation is misleading as it clearly states "Recursively converts an (observable) object to a javascript structure." - since "(observable)" is writting in parenthesis it gives the impression it would also recursively convert a non-observable object to a javascript structure in my second example. Do you see what I mean? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1557 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABvGhB4wPkrxQYY_NoN_IK9gArLd7DLtks5t1r8ggaJpZM4UMQDw> .

[capaj]

@stnwk I've ran into the same problem as you a year or so back. We should really patch that documentation there.

[wangyiz4262]

@stnwk @capaj I submitted PR to address the mentioned problem
Refactor toJS and Refactor toJS for mobx4

[y4ure]

@wangyiz4262 Thank you for you PR.
I've noticed though with the recurseEverything option it will throw when value is null (actually works for undefined). Is that the expected behaviour?

[wangyiz4262]

Hi @y4ure, thanks for noticing any potential bug! But I didn’t quite understand what you meant. What I can find is a null check in the beginning of the toJS function and that is from master branch which I didn’t change. Would you please elaborate a bit? Thanks a lot!

[y4ure]

Given that JSONs can contain null value, I'm expecting toJS to handle null values. However I'm getting this error below instead.

Uncaught TypeError: Cannot convert undefined or null to object
    at Function.getPrototypeOf (<anonymous>)
    at toJSHelper (D:\_Workspace\…bx\lib\mobx.js:2509)
    at toJSHelper (D:\_Workspace\…bx\lib\mobx.js:2528)
    at D:\_Workspace\…bx\lib\mobx.js:2503
    at Array.map (<anonymous>)
    at toJSHelper (D:\_Workspace\…bx\lib\mobx.js:2503)
    at toJS$$1 (D:\_Workspace\…bx\lib\mobx.js:2545)

[mweststrate]

@y4ure since the original issue is addressed, could you open a new issue for your problem, including a small reproduction in a codesandbox or as PR on the repository? Thanks!

[wangyiz4262]

@y4ure @mweststrate Two new PRs have been created to fix this issue, see #1783 for mobx5 and #1784 for mobx4.

[mweststrate]

Released as 4.5.2 / 5.5.2

[mxs42]

This behavior is still not covered in documentation.

[mweststrate]

PR welcome

…

On Mon, May 13, 2019 at 2:10 PM Max Kade ***@***.***> wrote: This behavior is still not covered in documentation. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1557?email_source=notifications&email_token=AAN4NBFPZYF67FN3HKBJ2KLPVFLE3A5CNFSM4FBRADYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVIDQPI#issuecomment-491796541>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAN4NBHESWOBQ66CZDK4DTLPVFLE3ANCNFSM4FBRADYA> .

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs or questions.