Fixed a couple of obscure replacer/reviver bugs. (#5)

* Fixed a couple of obscure replacer/reviver bugs.
* Fixed a reviver bug with non-primitive arguments of function types.

Author: kshetline

CHANGELOG.md

@@ -1,6 +1,14 @@
+### 4.2.3
+
+* Fixed a reviver bug with non-primitive arguments of function types.
+
+### 4.2.2
+
+* Fixed a couple of bugs related to performing deletions of array/object elements via replacer or reviver functions.
+
 ### 4.2.1
 
-* Documentation-only update.
+* Documentation-only update, clarifying replacer and reviver functions.
 
 ### 4.2.0
 

README.md

@@ -90,7 +90,7 @@ The following features, which are not supported in standard JSON, have been adde
 
 ### Replacer functions (JSON-Z specific differences)
 
-- When a replacer function returns `undefined` for the value of an item, for consistency with JSON replacer functions, that with cause the item to be deleted from the result. (`JSONZ.DELETE` can also be used for this purpose.)
+- When a replacer function returns `undefined` for the value of an item, for consistency with JSON replacer functions, that with cause the item to be deleted from the result. `JSONZ.DELETE` can also be used for this purpose, and *is the only way to delete an object or array item with a value that is originally `undefined`.*
 - Since JSON-Z can handle explicit `undefined` values, a replacer function can return the special value `JSONZ.UNDEFINED` to replace an item’s value with `undefined`.
 - Replacer functions can return the special value `JSONZ.DELETE` to indicate that a slot in an array be left empty, creating a sparse array.
 - A global replacer function can be specified.
@@ -215,9 +215,11 @@ The third argument passed to a JSON-Z reviver *might* be different from what a `
 > - `key`: The object key (or array index) of the value being parsed. The `key` is an empty string if the `value` is the root value.
 > - `value`: A value as originally parsed, which should be returned by the reviver as-is if the reviver is not modifying the original value.
 > - `extra`: This is either a `context` object containing a `source` string, as described at the link above, or the holder of the value, i.e., the object or array, if any, which contains the given key/value pair.
-> - `noContext`: if `true`, `extra` is the object or array which contains the current key/value pair. Otherwise, `value` is a primitive value, and `extra` functions like the (nearly) standard `context` object containing a `source` string, but also containing a `holder` value as well.
+> - `noContext`: if `true`, `extra` is the `holder` object or array which contains the current key/value pair. Otherwise, `value` is a primitive value, and `extra` functions like the (nearly) standard `context` object, containing a `source` string, but also containing a `holder` value as well.
 > 
-> Returns: Either the original `value`, or a modified value.
+> Returns: Either the original `value`, `JSONZ.DELETE`, or a modified value (using `JSONZ.UNDEFINED` to change a value to `undefined`).
+
+Please note that if you want to shrink the size of a containing array when using a reviver to delete an array element, you must both perform `holder.splice(parseInt(key), 1)` within the replacer and then return `JSONZ.DELETE` from the replacer function. Returning `JSONZ.DELETE` alone will simply result in a sparse array with an empty slot.
 
 #### Return value
 
@@ -239,8 +241,11 @@ This works very much like [`JSON.stringify`](https://developer.mozilla.org/en-US
 - `value`: The value to convert to a JSON-Z string.
 - `replacer`: A function which alters the behavior of the stringification process, or an array of String and Number objects that serve as an allowlist for selecting/filtering the properties of the value object to be included in the JSON-Z string. If this value is null or not provided, all properties of the object are included in the resulting JSON-Z string.
 
-  When using the standard `JSON.stringify()`, a replacer function is called with two arguments: `key` and `value`. JSON-Z adds a third argument, `holder`. This value is already available to standard replacer `function`s as `this`, but `this` won't be bound the holder when using an anonymous (arrow) function as a replacer. The JSON-Z third argument (which can be ignored if not needed) provides alternative access to the holder value.
-> `replacer(key, value[, holder])`
+  When using the standard `JSON.stringify()`, a replacer function is called with two arguments: `key` and `value`. JSON-Z adds a third argument, `holder`. This value is already available to standard replacer `function`s as `this`, but `this` won't be bound the holder when using an anonymous (arrow) function as a replacer. The JSON-Z third argument (which can be ignored if not needed) provides alternative access to the holder value.<br><br>
+  > `replacer(key, value[, holder])`
+
+  <br>Please note that if you want to shrink the size of a containing array when using a replacer to delete an array element, you must both perform `‑‑holder.length` within the replacer and then return `JSONZ.DELETE` from the replacer function. Returning `JSONZ.DELETE` alone will simply result in a sparse array with an empty slot.
+  
 - `space`: A string or number used to insert whitespace into the output JSON-Z string for readability purposes. If this is a number, it indicates the number of space characters to use as whitespace; this number is capped at 10. Values less than 1 indicate that no space should be used. If `space` is a string, that string (or the first 10 characters of the string if it's longer) is used as white space. A single space adds white space without adding indentation. If this parameter is not provided (or is null), no whitespace is added. If indenting white space is used, trailing commas can optionally appear in objects and arrays.
 - `options`: This can either be an `OptionSet` value (see [below](#jsonzsetoptionsoptions-additionaloptions)), or an object with the following properties:
   - `extendedPrimitives`: If `true` (the default is `false`) this enables direct stringification of `Infinity`, `-Infinity`, `NaN`, and `undefined`. Otherwise, these values become `null`.

lib/options-manager.js

@@ -1,6 +1,7 @@
 const big = require('./bignumber-util');
 const util = require('./util');
 const platform = require('./platform-specifics');
+const { ValueSourceWrapper } = require('./util');
 
 let globalOptions = {};
 let globalParseOptions = {};
@@ -254,6 +255,11 @@ module.exports = {
     let typeName;
     let container = false;
 
+    /* istanbul ignore next */ // This isn't currently being hit, but I feel better leaving it in for safety.
+    if (value instanceof ValueSourceWrapper) {
+      value = value.value;
+    }
+
     if (util.isTypeContainer(typeNameOrContainer)) {
       container = true;
       typeName = typeNameOrContainer._$_;

lib/parse.js

@@ -82,7 +82,6 @@ module.exports = function parse(text, reviver, newOptions) {
     return newPrimitiveToken('number', NaN);
   }
 
-  // noinspection JSUnusedGlobalSymbols (many functions here are accessed by string content, not direct in-code references)
   const lexStates = {
     default() {
       switch (c) {
@@ -1203,13 +1202,15 @@ module.exports = function parse(text, reviver, newOptions) {
 
     if (value && typeof value === 'object' && !isBigNumber(value) && !(value instanceof ValueSourceWrapper)) {
       // noinspection JSUnresolvedReference
-      const keys = Object.keys(value);
+      const keys = Object.keys(value).reverse();
 
       for (const key of keys) {
         const replacement = internalize(value, key, reviver);
 
         if (replacement === undefined || replacement === util.DELETE) {
-          delete value[key];
+          if (!Array.isArray(value) || key < value.length - 1) {
+            delete value[key];
+          }
         }
         else {
           value[key] = replacement === util.UNDEFINED ? undefined : replacement;
@@ -1556,7 +1557,9 @@ module.exports = function parse(text, reviver, newOptions) {
       let revived;
 
       try {
-        revived = optionsMgr.reviveTypeValue(current.name, current.arg);
+        const arg = typeof reviver === 'function' ? internalize({ '': current.arg }, '', reviver) : current.arg;
+
+        revived = optionsMgr.reviveTypeValue(current.name, arg);
       }
       catch (err) {
         throw syntaxError(err.message, true);

lib/stringify.js

@@ -153,6 +153,8 @@ module.exports = function stringify(value, replacer, space) {
       value = value.toJSON(key);
     }
 
+    const originalValue = value;
+
     if (replacerFunc) {
       value = replacerFunc.call(holder, key, value, holder);
     }
@@ -169,9 +171,9 @@ module.exports = function stringify(value, replacer, space) {
       case null: return 'null';
       case true: return 'true';
       case false: return 'false';
-      case undefined: return extendedPrimitives ? 'undefined' : 'null';
+      case undefined: return originalValue !== undefined ? util.DELETE : extendedPrimitives ? ('undefined') : 'null';
       case util.DELETE: return util.DELETE;
-      case util.UNDEFINED: return util.UNDEFINED;
+      case util.UNDEFINED: return 'undefined';
     }
 
     if (typeof value === 'string') {
@@ -311,15 +313,14 @@ module.exports = function stringify(value, replacer, space) {
 
     const partial = [];
 
-    for (let i = 0; i < value.length; i++) {
+    for (let i = value.length - 1; i >= 0; --i) {
       if (!sparseArrays || i in value) {
         const propertyString = serializeProperty(String(i), value, true);
 
         if (propertyString === util.DELETE) {
-          partial.push('');
-        }
-        else if (propertyString === util.UNDEFINED) {
-          partial.push('undefined');
+          if (i < value.length) {
+            partial.push('');
+          }
         }
         else {
           partial.push(propertyString);
@@ -330,6 +331,8 @@ module.exports = function stringify(value, replacer, space) {
       }
     }
 
+    partial.reverse();
+
     if (revealHiddenArrayProperties) {
       const keys = Object.keys(value);
 

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "json-z",
-  "version": "4.2.1",
+  "version": "4.2.3",
   "description": "JSON for everyone.",
   "main": "lib/index.min.js",
   "types": "lib/index.d.ts",

package.json

@@ -571,18 +571,18 @@ it('parse(text, reviver)', () => {
   expect(
     JSONZ.parse('{a:1,b:2}', (k, v) => (k === 'a') ? JSONZ.DELETE : v)).to.deep.equal(
     { b: 2 },
-    'deletes property values'
+    'JSONZ.DELETE deletes property values'
   );
 
   expect(
     JSONZ.parse('{a:1,b:2}', (k, v) => (k === 'a') ? undefined : v)).to.deep.equal(
     { b: 2 },
-    'also deletes property values'
+    '`undefined` deletes property values'
   );
 
   expect(
-    JSONZ.parse('{a:1,b:2}', (k, v) => (k === 'a') ? JSONZ.UNDEFINED : v)).to.deep.equal(
-    { a: undefined, b: 2 },
+    JSONZ.parse('{a:1,b:2,c:_BigInt(3),d:_Map([[4,5]])}', (k, v) => (k === 'a') ? JSONZ.UNDEFINED : v)).to.deep.equal(
+    { a: undefined, b: 2, c: 3n, d: new Map([[4, 5]]) },
     'replaces property values with `undefined`'
   );
 
@@ -611,6 +611,20 @@ it('parse(text, reviver)', () => {
     'deletes array values'
   );
 
+  expect(
+    JSONZ.parse('[0,1,2]', (k, v, context) => {
+      if (k === '1') {
+        context.holder.splice(parseInt(k), 1);
+        return JSONZ.DELETE;
+      }
+      else {
+        return v;
+      }
+    })).to.deep.equal(
+    [0, 2],
+    'deletes array values and shrinks array'
+  );
+
   expect(
     JSONZ.parse('33', () => JSONZ.DELETE)).to.equal(
     undefined,

test/parse.spec.mjs

@@ -491,24 +491,34 @@ describe('stringify', () => {
       );
     });
 
-    it('deletes object values when a replacer returns DELETE', () => {
+    it('deletes object values when a replacer returns DELETE or `undefined`', () => {
       assert.strictEqual(
         JSONZ.stringify({ a: 1, b: 2 }, (key, value) => (key === 'b') ? JSONZ.DELETE : value),
         '{a:1}'
       );
+
+      assert.strictEqual(
+        JSONZ.stringify({ a: 1, b: 2, c: undefined }, (key, value) => (key && key !== 'a') ? undefined : value),
+        '{a:1,c:undefined}'
+      );
+
+      assert.strictEqual(
+        JSONZ.stringify({ a: 1, b: 2, c: undefined }, (key, value) => (key && key !== 'a') ? JSONZ.DELETE : value),
+        '{a:1}'
+      );
     });
 
     it('can transform object values into undefined values with replacer', () => {
       assert.strictEqual(
-        JSONZ.stringify({ a: 1, b: 2 }, (key, value) => (key === 'b') ? undefined : value),
+        JSONZ.stringify({ a: 1, b: 2 }, (key, value) => (key === 'b') ? JSONZ.UNDEFINED : value),
         '{a:1,b:undefined}'
       );
     });
 
     it('creates empty array slots when a replacer returns DELETE', () => {
       assert.strictEqual(
-        JSONZ.stringify([1, 77, 3], (key, value) => (value === 77) ? JSONZ.DELETE : value),
-        '[1,,3]'
+        JSONZ.stringify([1, 77, 3, undefined], (key, value) => (value === 77) ? JSONZ.DELETE : value),
+        '[1,,3,undefined]'
       );
     });
 
@@ -519,10 +529,30 @@ describe('stringify', () => {
       );
     });
 
-    it('can transform array values into undefined values with replacer', () => {
+    it('creates empty array slots when a replacer returns `undefined`, unless original value is `undefined`', () => {
       assert.strictEqual(
-        JSONZ.stringify([1, 77, 3], (key, value) => (value === 77) ? undefined : value),
-        '[1,undefined,3]'
+        JSONZ.stringify([1, 77, 3, undefined], (key, value) => (key > 1) ? undefined : value),
+        '[1,77,,undefined]'
+      );
+
+      assert.strictEqual(
+        JSONZ.stringify([1, 77, 3, undefined], (key, value) => (key > 1) ? JSONZ.DELETE : value),
+        '[1,77,,]'
+      );
+    });
+
+    it('can shrink array by manipulating holder', () => {
+      assert.strictEqual(
+        JSONZ.stringify([1, 77, 3, undefined], (key, value, holder) => {
+          if (key > 1) {
+            --holder.length;
+            return JSONZ.DELETE;
+          }
+          else {
+            return value;
+          }
+        }),
+        '[1,77]'
       );
     });
 

test/stringify.spec.mjs

@@ -5,7 +5,15 @@
     "emitDeclarationOnly": true,
     "outDir": "types",
     "noEmit": false,
-    "skipLibCheck": true
+    "skipLibCheck": true,
+    "target": "es2020",
+    "types": [
+      "node"
+    ],
+    "lib": [
+      "es2020",
+      "dom"
+    ]
   },
   "include": ["lib/index.d.ts"],
   "exclude": ["node_modules", "dist"]