search

tlrobinson / narwhal

[DEPRECATED] A JavaScript standard library, package manager, and more.
http://narwhaljs.org/
372 stars 16 forks source link

Avoids overwriting native JSON methods #72

Closed kriszyp closed 14 years ago

kriszyp commented 14 years ago

We should not be overwriting native JSON methods if they are available. http://github.com/kriszyp/narwhal/commit/f8fccee637d9165c935e6274fa96143e7589ec93

tlrobinson commented 14 years ago

Should we use global.JSON or typeof "JSON" instead of this.JSON?

kriszyp commented 14 years ago

global.JSON is fine, I guess I was just trying to minimize the amount of code I changed.

tlrobinson commented 14 years ago

SHA: 642e09f8a8213d1b18dd71170853a9a1808f8415 SHA: 9908a418c125ffc58046dd01237e8d62f7e25eb9

kriszyp commented 14 years ago

It looked like this was reverted, was there an issue with this change (perhaps Rhino's trailing whitespace issue)?

tlrobinson commented 14 years ago

Yeah I had to revert it because something was failing, but I didn't have time to investigate why. I'm not familiar with Rhino's trailing whitespace issue but that sounds plausible.

tlrobinson commented 14 years ago

Actually it was weirder than that. There were instances of

var json = require("json")
json.parse(whatever)

which threw a "json.parse is undefined" error, even though it worked in the REPL. I think it was in tusk.

I'm not clear on when we should be using require("json") vs the global JSON object?