Exception in JSON.stringify

[sjpt]

Description of the problem

Sample code:
mmm = new THREE.Mesh(); JSON.stringify(mmm);
throws exception
[(http://jsfiddle.net/sjpt/akmcv7Lh/32/)]

My larger scale code is stringifying at a higher level, and tries to avoid any attempt to stringify the THREE components by
JSON.stringify(o, function(key, value) { if value instanceof THREE.Object3D return undefined; ... }

This worked in older versions of THREE. However, the THREE.Object3D.prototype.toJSON code now gets called BEFORE my escape function, and the exception means my entire JSON.stringify fails, even though it didn't want to go near stringifying the THREE objects.

For now I have a nasty workaround of
delete THREE.Object3D.prototype.toJSON;
before calling the stringify, but that is obviously not a good long term or general solution.

Three.js version

• Dev
• r75
• ...

Browser

• All of them
• Chrome
• Firefox
• Internet Explorer

OS

• All of them
• Windows
• Linux
• Android
• IOS

Hardware Requirements (graphics card, VR Device, ...)

[mrdoob]

Shouldn't you be doing JSON.stringify( mmm.toJSON() ) instead?

[sjpt]

Thank you for looking at this so quickly. I apologize for the incomplete example. I have updated the jsfiddle so it more fully illustrates the issue. http://jsfiddle.net/sjpt/akmcv7Lh/40/

[pietro909]

Hi @sjpt I had a look at your fiddle and the issue is not related to THREE: the only explanation I can find is that the replacer function of JSON.stringify is being called after the process has been executed for that value.
This is why you always run into the error.
My suggestion is filtering it before.

[sjpt]

I am afraid it is related to THREE, as it is THREE that is throwing the exception and causing failure of what should be a safe call to JSON.stringify.

I agree it would be more convenient if my replacer() filter were called before the THREE toJSON() method, which would be more efficient and in which case the THREE bug would not matter. However, the spec at http://www.ecma-international.org/ecma-262/6.0/#sec-serializejsonproperty indicates this is the correct behaviour. As I said at in the first post I have a workaround so it is not too important for me, however, I think the THREE bug should be fixed.

[tschw]

@mrdoob

Shouldn't you be doing JSON.stringify( mmm.toJSON() ) instead?

JSON.stringify calls .toJSON automagically. One could say we use a reserved name.

You easily get exceptions (at best - weird behavior being the worse case) from within the JSON serializer, since it doesn't pass our extra meta parameter.

For instance, I know that this line throws when trying to serialize a ShaderMaterial with texture uniforms (current workaround: Set .values to null and re-add the textures manually after the scene has been loaded).

[mrdoob]

Oh noes! Wasn't aware... Is the solution renaming to .serialize()?

[tschw]

@mrdoob

Oh noes! Wasn't aware...

Me not either until I ran into it.

Is the solution renaming to .serialize()?

Arguably. As long as our implementation is complete, there's no problem. In fact, one could see it as syntactic sugar. OTOH it avoids confusion like here.

What OP wants (specifying a replacer argument to JSON.stringify) requires it to be called toJSON. We can't support that, unless putting meta somewhere global, but that (is ugly to begin with and) raises the question when to get rid of it again.

I consider serializing ShaderMaterial "partly unimplemented" (for many other reasons, not just uniforms), that's why I didn't file a bug - thought I better come up with a solution at some point.

[sjpt]

I think your renaming Object3D.toJSON is a good solution. toJSON is a reserved method name with behaviour specified in the JSON.serialize specification; and that behaviour involves calling the method without arguments. If you want to implement something similar but different, your method should have a different name. That presumably means a rename of your other toJSON methods too.

I do not need your serializer to be called as I ignore its output anyway in my replacer function. I just need a serializer that does not throw an exception. The default serializer (which gets called as a result of my delete THREE.Object3D.prototype.toJSON; workaround satisfies that. I don't know what output the default serializer has produced for your objects or whether there is any context where it might be useful, but at least it doesn't break things by throwing an exception.

I could save a trivial amount of compute time by replacing my workaround with
THREE.Object3D.prototype.toJSON = function() { return undefined; }, but it would still be a workaround that could break other things. I don't see any downside to renaming your toJSON method(s).

[mrdoob]

I could see some potential breakage. People that relied on toJSON() will now get a different output and I'm not sure we can add a console.warn()... However, I think the breakage needs to be done.

[sjpt]

One suggestion to minimize breakage is to provide a toJSON that detects whether it has an input argument. Curiously, it seems that JSON.serialize calls any toJSON method with a single argument of an empty string, at least on Chrome and Firefox. I'm not sure if this is safe specified behaviour.

toJSON: function ( meta ) {
if (meta === "") return this;
console.warn(...);
... rest of code as before
}
seems to work. (I was afraid it might go into a recursive loop.)

[tschw]

How about just letting it break and adding an IMPORTANT sentence to the release notes?

[tschw]

@mrdoob

Is the solution renaming to .serialize()?

@tschw

Arguably. As long as our implementation is complete, there's no problem. In fact, one could see it as syntactic sugar.

To clarify: JSON.stringify should never see a .toJSON method except for the top-level object when the object is passed in directly.

To make this case work, we'd have to change this line to

var isRootObject = ( meta === undefined || meta === '' );

or use ! meta for once to be on the safe side. It would even work with replacers, but those would only be called on the root (and unimplemented stuff), because that's how our serialization works.

[sjpt]

The same issue has returned with THREE.Texture(). Not at all serious for me as I am now using yaml instead of JSON.