bestiejs / json3

A JSON polyfill. No longer maintained.
https://bestiejs.github.io/json3
Other
1.02k stars 150 forks source link

Prototype <= 1.6.1 Support #8

Closed ghost closed 12 years ago

ghost commented 12 years ago

So...here's the story behind JSON 3 not supporting Prototype <= 1.6.1.

According to the spec entry for stringify, the toJSON method is used to obtain an alternative object for serialization. For example, Backbone's Model#toJSON returns a plain object containing the model's attributes, which is then serialized using the algorithm defined in the spec. For this behavior to make sense, toJSON should return a string, number, Boolean, array literal, or object literal—the only objects for which the serialization routine is explicitly defined.

Unfortunately, older versions of Prototype dictate that the toJSON method should return a pre-serialized JSON string. To make matters worse, it also defines toJSON methods on strings, dates, numbers, and arrays (but not booleans)—which means that stringify will call them to obtain a replacement value (a pre-serialized string), and then serialize that value. The result will always be a string literal, rather than a proper JSON structure.

This behavior is actually consistent with the spec, and using the native implementation will yield identical results. We can confirm this by running the following snippet in a browser that supports native JSON, and without JSON 3 loaded:

(function() {
  var script = document.createElement("script");
  script.src = "http://prototypejs.org/assets/2009/8/31/prototype.js";
  script.onload = function () {
    // This should be `true`.
    alert(JSON.stringify([1, 2, 3]) == "\"[1, 2, 3]\"");
  };
  document.body.appendChild(script);
}());

The spec doesn't account for a library that completely redefines how toJSON works, and catering to these versions of Prototype would require breaking compatibility with the spec. The objective of JSON 3 is twofold: provide a spec-complaint polyfill for older browsers, and fix bugs in the native JSON implementations of newer ones. I'm afraid that fixing a buggy, dated library (1.6.1 was released in September 2009; 1.7, which corrects the issues, was released in November 2010) would fall outside the scope of this project.

A Note on Object.toJSON: The Object.toJSON method provided by Prototype is not an appropriate fallback for JSON.stringify—it doesn't support the filter or whitespace arguments, and will recurse indefinitely when attempting to serialize cyclic structures. Additionally, Prototype's date serialization is broken (it doesn't support extended years or milliseconds, for example), and doesn't work around the bugs present in Opera >= 10.53.

The only viable option that I can see is to monkey-patch Prototype. There are two significant problems with this approach as well. First, it would involve increasing the size and complexity of the source for a very specific audience. Secondly, even if all of Prototype's buggy methods could be monkey-patched, nothing can be done about third-party code. Remember that Prototype redefines the behavior of toJSON—which means that third-party code that relies on this behavior will break if the library is patched. It's a less than stellar solution.

I'm certainly open to discussing this, but let's keep in mind that any solution we come up with is likely to be an egregious hack. For that reason, I'd like to tread cautiously before implementing any kind of a patch (@tobie, @savetheclocktower—if you have the time and inclination, I'd love to hear any thoughts that you may have about this).

tobie commented 12 years ago

Ya, here's the backstory to this unfortunate issue: https://groups.google.com/group/prototype-core/msg/76508c7dae8be2a1

Nothing much to be done. Prototype <= 1.6.1 was designed around Crockford's initial JSON implementation which differed completely.

jdalton commented 12 years ago

I've had libs that are loaded in pages first and handle the Prototype issue by doing something like

;(function(window) {
  var JSON = {
    'parse': window.JSON.parse,
    'stringify': window.JSON.stringify
  };
  // ...
  function stringify(value, replacer, space) {
    // backup `toJSON` methods
    var key = 'toJSON';
    var backups = [
      [Array,   hasOwnProperty.call(ArrayProto, key) && isFunction(ArrayProto[key]) && ArrayProto[key]],
      [Boolean, hasOwnProperty.call(BoolProto, key)  && isFunction(BoolProto[key])  && BoolProto[key]],
      [Object,  hasOwnProperty.call(ObjProto, key)   && isFunction(ObjProto[key])   && ObjProto[key]],
      [Number,  hasOwnProperty.call(NumProto, key)   && isFunction(NumProto[key])   && NumProto[key]],
      [String,  hasOwnProperty.call(StrProto, key)   && isFunction(StrProto[key])   && StrProto[key]]
    ];
    // delete `toJSON` methods 
    forEach(backups, function(pair){
      pair[1] && delete pair[0].prototype[key];
    });
    // get stringify result
    var result = JSON.stringify(value, replacer, space);
    // restore `toJSON` methods
    forEach(backups, function(pair){
      pair[1] && (pair[0].prototype[key] = pair[1]);
    });
    return result;
  }
  // ...
}(this));

That said, I don't think its JSON3's problem to fix an old lib's bad code. The devs using Prototype should be well aware by now of its issues/limitations.

ghost commented 12 years ago

@tobie Thanks very much for chiming in. I've archived your Google Groups post as a Gist for future reference, and added a link to the GitHub Page.

@jdalton That's a great patch—might I suggest surrounding the call to JSON.stringify with a try...catch, though? That way, the toJSON methods will be restored even if stringify throws an error. It doesn't address third-party code (I'm not sure if that's even possible), but should be more than adequate for most circumstances.

I definitely agree that this doesn't belong in the core. The fact that the native implementation produces identical results is reason enough to keep the current behavior. Users of Prototype <= 1.6.1 can continue to use Object.toJSON and String#evalJSON—no need for JSON 3.

ghost commented 12 years ago

I've implemented a simple fix for this in a326fcd8540796c5472df99ebdfc9222b4a73f61. stringify will now ignore inherited toJSON methods on numbers, strings, dates, and arrays. It's far from perfect, but should take care of the majority of compatibility issues.

Pinging @oyvindkinsey—does this look okay to you?

oyvindkinsey commented 12 years ago

Hey @tobie, cool to see you here :)

@kitcambridge, yeah - this seems to be logically consistent - thanks for doing this!

ghost commented 12 years ago

@oyvindkinsey Thank you for the review!