Bug with JSON.stringify

[Benoît Léo Maillard]

Hi,

I've experienced another bug with one of my scripts. RapydScript redefines JSON.stringify() function (I can't figure out why...) and there is a mistake in the way this function handles "\" (backslash).

Here is an example:

o = {"key" : "\\"}; // 

console.log(JSON.stringify(o));

Result with native Javascript: "{"key":"\\"}"

Result with RapydScript's JSON.stringify: "{"key":"\"}"

With RapydScript, one backslash is missing and JSON.stringify produces invalid json (check it on http://jsonlint.com/)

I can't use MathJax in my app because I need to enter some formulas like \frac{4}{3} and then send them to the server as a json string.

I think redefining standard functions is a bad practice anyway, you should define an other function if JSON.stringify doesn't fulfil your needs.

Thank you for your attention,

Best regards

[Charles Law]

Good catch, it does look like there is a bug with the stringify implementation.

It shouldn't be overwriting the default JSON stringify though.  Any chance you're using an old version?  How are you compiling? or are importing including stdlib.js somehow (via html?)?

I just ran with 0.2.10 and 0.2.11 (latest) and with this input:

import stdlib

o = {"key" : "\\"}

console.log(JSON.stringify(o))

I got this output:

...

JSON = JSON || {};

if (!JSON.stringify) {

    JSON.stringify = function(obj) {

...

o = {

    "key": "\\"

};

console.log(JSON.stringify(o));

When I ran the code, everything output as expected.

[Benoît Léo Maillard]

It still doesn't work.

I've found the mistake:

...

var JSON, str; // creates a new local variable named JSON (undefined)

JSON = JSON || {}; // JSON will be an empty object

if (!JSON.stringify) { // The function is redefined

...

In source code (RapydScript/blob/master/src/lib/stdlib_common.pyj l.14),

...

JSON = JSON or {}

# implement JSON.stringify in older browsers, if doesn't already exist

if not JSON.stringify:

...

should just be replaced by:

...

# implement JSON.stringify in older browsers, if doesn't already exist

if typeof(JSON) == "undefined":

...

Then it works great ;)

[Alexander Tsepkov]

Charles, add "nonlocal" before JSON. Thanks for catching the bug, Benoît.

[Charles Law]

I made a PR to fix the JSON.stringify implementation and added nonlocal for JSON.  I wasn't seeing var JSON in my code, but it looks like adding nonlocal doesn't affect it on my machine, and should hopefully fix it for others:

https://github.com/atsepkov/RapydScript/pull/35

Are there any docs on contributing?  I can add some tests sometime, but I wasn't sure about recompiling the code output in the lib folder (thought I don't think that matters for this PR).

[Alexander Tsepkov]

Thanks Charles, I can add the tests later. I'll wait with the PR until I get a chance to test this tomorrow - since there is a large chunk of code changing. On a related note, do you remember the reason for us using JSON.stringify rather than String() directly? I feel like we examined this before but don't remember why we didn't use String(). All other conversions make use of regular JS converters:

int() = parseInt()

float() = parseFloat()

bool() = !!()

str() = JSON.stringify()

Also, since JS has the concept of "String" vs "new String" (it's features like this that make me want to avoid pure JS), I tested a few current ways of calling "String" from within RapydScript, and was a bit surprised to see 2/4 of the ways fail. I will need to submit a fix for these over the weekend as well

a = JS("String(stuff_to_convert)")            # works

a = String.call(this, stuff_to_convert)       # broken, call should be a "static" method on String (along with apply, etc.)

a = (String)(1+2)                                      # broken, String without () should not trigger a 'new'

toStr = String; a = toStr(stuff_to_convert)    # works

[Charles Law]

I don't recall why clearly.  I *think* it may have been for hash/dicts/Objects.  Something like:

String({'a': 2})

"[object Object]"

vs

JSON.stringify({'a': 2})

"{"a":2}"

but I'm not 100% sure.

[Alexander Tsepkov]

Yep, that was it. I guess we'll keep it as JSON.stringify.

[Charles Law]

It probably makes sense to hold off on this PR until those fixes are in.  I have a few JS('String(stuff)') I'd like to replace with (String)(stuff).  Also, I can make use of the for n in dir(obj) you pointed out to me earlier to remove another JS().

I can make a simple PR for the nonlocal change since that's a small and straightforward change.

[Alexander Tsepkov]

I've fixed both methods of calling String without 'new' now.

[Charles Law]

Just made the PR to fix JSON.stringify including tests.