Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

qx.data.marshal.Json | ignore non-pojo objects when marshalling #10702

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
18 changes: 16 additions & 2 deletions source/class/qx/Bootstrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -867,8 +867,8 @@ qx.Bootstrap.define("qx.Bootstrap", {
},

/**
* Whether the value is an object. Note that built-in types like Window are
* not reported to be objects.
* Whether the value is an object i.e. Object.prototype is its prototype or Object.prototype in its prototype chain.
* Note that built-in types like Window are not reported to be objects.
*
* @param value {var} Value to check.
* @return {Boolean} Whether the value is an object.
Expand All @@ -881,6 +881,20 @@ qx.Bootstrap.define("qx.Bootstrap", {
);
},

/**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isPojo does not need to be in qx.Bootstrap, which is effectively an internal class that exists in order to work around dependency management issues - IMHO it should move to qx.lang.Type

* Whether the value is strictly a POJO. It's prototype must not inherit from Object.prototype but be strictly Object.prototype.

* @param {*} value
* @returns {boolean} Whether the value is strictly a POJO.
*/
isPojo(value) {
return (
value !== undefined &&
value !== null &&
Object.getPrototypeOf(value) === Object.prototype
);
},

/**
* Whether the value is a function.
*
Expand Down
10 changes: 5 additions & 5 deletions source/class/qx/data/marshal/Json.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,9 +170,9 @@ qx.Class.define("qx.data.marshal.Json", {
* @param depth {Number} The depth of the data relative to the data's root.
*/
__toClass(data, includeBubbleEvents, parentProperty, depth) {
// break on all primitive json types and qooxdoo objects
// break on all primitive json types and non-POJO objects
if (
!qx.lang.Type.isObject(data) ||
!qx.lang.Type.isPojo(data) ||
!!data.$$isString || // check for localized strings
data instanceof qx.core.Object
) {
Expand Down Expand Up @@ -405,12 +405,12 @@ qx.Class.define("qx.data.marshal.Json", {
* @return {qx.core.Object} The created model object.
*/
__toModel(data, includeBubbleEvents, parentProperty, depth) {
var isObject = qx.lang.Type.isObject(data);
var isPojo = qx.lang.Type.isPojo(data);
var isArray =
data instanceof Array || qx.Bootstrap.getClass(data) == "Array";

if (
(!isObject && !isArray) ||
(!isPojo && !isArray) ||
!!data.$$isString || // check for localized strings
data instanceof qx.core.Object
) {
Expand Down Expand Up @@ -451,7 +451,7 @@ qx.Class.define("qx.data.marshal.Json", {
);
}
return array;
} else if (isObject) {
} else if (isPojo) {
// create an instance for the object
var hash = this.__jsonToBestHash(data, includeBubbleEvents);
var model = this.__createInstance(hash, data, parentProperty, depth);
Expand Down
14 changes: 11 additions & 3 deletions source/class/qx/lang/Type.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,23 @@ qx.Bootstrap.define("qx.lang.Type", {
isArray: qx.Bootstrap.isArray,

/**
* Whether the value is an object. Note that built-in types like Window are
* not reported to be objects.
* Whether the value is an object i.e. Object.prototype is its prototype or Object.prototype is in its prototype chain.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be helpful to explain the net effect - eg: "Objects can be either a POJO (ie {}) or an object which is created from a ES6-style class or prototypical-inheritance-based class; if you need to determine whether something is a POJO and not created from a class, use isPojo instead"

* Note that built-in types like Window are not reported to be objects.
*
* @signature function(value)
* @param value {var} Value to check.
* @param {*} value value to check.
* @return {Boolean} Whether the value is an object.
*/
isObject: qx.Bootstrap.isObject,

/**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, the net effect is more useful in documentation than repeating the code, eg "Detects whether an object is just a plain POJO, ie {}; note that this excludes objects created via classes - @see qx.lang.Type.isObject

* Whether the value is strictly a POJO. It's prototype must not inherit from Object.prototype but be strictly Object.prototype.
* @signature function(value)
* @param {*} value
* @returns {Boolean} Whether the value is strictly a POJO.
*/
isPojo: qx.Bootstrap.isPojo,

/**
* Whether the value is a function.
*
Expand Down
Loading