Skip to content

Commit

Permalink
Support diffing keys named like Object.prototype properties
Browse files Browse the repository at this point in the history
Fixes #58.

Previously, the code assumed that input objects have a `hasOwnProperty`
function, or that at least they will acquire one when passed through
`utils.properObject`.

However, this assumption is flawed:  As noted in issue #58, when given
input objects have a property on them called `hasOwnProperty`, it
overrides the prototype's function property that the code relied on,
causing any diffing function to error out with

    Uncaught TypeError: r.hasOwnProperty is not a function

The solution taken here is to forget about `utils.properObject`, and
instead introduce `utils.hasOwnProperty` which uses
`Object.prototype.hasOwnProperty.call` instead of assuming anything
about the input object, and replacing all direct
`inputObject.hasOwnProperty` calls with it instead.
  • Loading branch information
anko authored and mattphillips committed Jan 25, 2022
1 parent ca791b3 commit b281292
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 54 deletions.
8 changes: 4 additions & 4 deletions src/added/index.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import { isEmpty, isObject, properObject } from '../utils';
import { isEmpty, isObject, hasOwnProperty } from '../utils';

const addedDiff = (lhs, rhs) => {

if (lhs === rhs || !isObject(lhs) || !isObject(rhs)) return {};

const l = properObject(lhs);
const r = properObject(rhs);
const l = lhs;
const r = rhs;

return Object.keys(r).reduce((acc, key) => {
if (l.hasOwnProperty(key)) {
if (hasOwnProperty(l, key)) {
const difference = addedDiff(l[key], r[key]);

if (isObject(difference) && isEmpty(difference)) return acc;
Expand Down
14 changes: 7 additions & 7 deletions src/arrayDiff/index.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import { isDate, isEmpty, isObject, properObject } from '../utils';
import { isDate, isEmpty, isObject, hasOwnProperty } from '../utils';

const diff = (lhs, rhs) => {
if (lhs === rhs) return {}; // equal return no diff

if (!isObject(lhs) || !isObject(rhs)) return rhs; // return updated rhs

const l = properObject(lhs);
const r = properObject(rhs);
const l = lhs;
const r = rhs;

const deletedValues = Object.keys(l).reduce((acc, key) => {
return r.hasOwnProperty(key) ? acc : { ...acc, [key]: undefined };
return hasOwnProperty(r, key) ? acc : { ...acc, [key]: undefined };
}, {});

if (isDate(l) || isDate(r)) {
Expand All @@ -19,11 +19,11 @@ const diff = (lhs, rhs) => {

if (Array.isArray(r) && Array.isArray(l)) {
const deletedValues = l.reduce((acc, item, index) => {
return r.hasOwnProperty(index) ? acc.concat(item) : acc.concat(undefined);
return hasOwnProperty(r, index) ? acc.concat(item) : acc.concat(undefined);
}, []);

return r.reduce((acc, rightItem, index) => {
if (!deletedValues.hasOwnProperty(index)) {
if (!hasOwnProperty(deletedValues, index)) {
return acc.concat(rightItem);
}

Expand All @@ -40,7 +40,7 @@ const diff = (lhs, rhs) => {
}

return Object.keys(r).reduce((acc, key) => {
if (!l.hasOwnProperty(key)) return { ...acc, [key]: r[key] }; // return added r key
if (!hasOwnProperty(l, key)) return { ...acc, [key]: r[key] }; // return added r key

const difference = diff(l[key], r[key]);

Expand Down
8 changes: 4 additions & 4 deletions src/deleted/index.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { isEmpty, isObject, properObject } from '../utils';
import { isEmpty, isObject, hasOwnProperty } from '../utils';

const deletedDiff = (lhs, rhs) => {
if (lhs === rhs || !isObject(lhs) || !isObject(rhs)) return {};

const l = properObject(lhs);
const r = properObject(rhs);
const l = lhs;
const r = rhs;

return Object.keys(l).reduce((acc, key) => {
if (r.hasOwnProperty(key)) {
if (hasOwnProperty(r, key)) {
const difference = deletedDiff(l[key], r[key]);

if (isObject(difference) && isEmpty(difference)) return acc;
Expand Down
10 changes: 5 additions & 5 deletions src/diff/index.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import { isDate, isEmptyObject, isObject, properObject } from "../utils";
import { isDate, isEmptyObject, isObject, hasOwnProperty } from '../utils';

const diff = (lhs, rhs) => {
if (lhs === rhs) return {}; // equal return no diff

if (!isObject(lhs) || !isObject(rhs)) return rhs; // return updated rhs

const l = properObject(lhs);
const r = properObject(rhs);
const l = lhs;
const r = rhs;

const deletedValues = Object.keys(l).reduce((acc, key) => {
return r.hasOwnProperty(key) ? acc : { ...acc, [key]: undefined };
return hasOwnProperty(r, key) ? acc : { ...acc, [key]: undefined };
}, {});

if (isDate(l) || isDate(r)) {
Expand All @@ -18,7 +18,7 @@ const diff = (lhs, rhs) => {
}

return Object.keys(r).reduce((acc, key) => {
if (!l.hasOwnProperty(key)) return { ...acc, [key]: r[key] }; // return added r key
if (!hasOwnProperty(l, key)) return { ...acc, [key]: r[key] }; // return added r key

const difference = diff(l[key], r[key]);

Expand Down
4 changes: 2 additions & 2 deletions src/preserveArray.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { isObject } from './utils';
import { isObject, hasOwnProperty } from './utils';

const getLargerArray = (l, r) => l.length > r.length ? l : r;

Expand All @@ -16,7 +16,7 @@ const preserve = (diff, left, right) => {
return {
...acc,
[key]: array.reduce((acc2, item, index) => {
if (diff[key].hasOwnProperty(index)) {
if (hasOwnProperty(diff[key], index)) {
acc2[index] = preserve(diff[key][index], leftArray[index], rightArray[index]); // diff recurse and check for nested arrays
return acc2;
}
Expand Down
8 changes: 4 additions & 4 deletions src/updated/index.js
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
import { isDate, isEmptyObject, isObject, properObject } from "../utils";
import { isDate, isEmptyObject, isObject, hasOwnProperty } from '../utils';

const updatedDiff = (lhs, rhs) => {
if (lhs === rhs) return {};

if (!isObject(lhs) || !isObject(rhs)) return rhs;

const l = properObject(lhs);
const r = properObject(rhs);
const l = lhs;
const r = rhs;

if (isDate(l) || isDate(r)) {
if (l.valueOf() == r.valueOf()) return {};
return r;
}

return Object.keys(r).reduce((acc, key) => {
if (l.hasOwnProperty(key)) {
if (hasOwnProperty(l, key)) {
const difference = updatedDiff(l[key], r[key]);

// If the difference is empty, and the lhs is an empty object or the rhs is not an empty object
Expand Down
8 changes: 4 additions & 4 deletions src/utils/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
export const isDate = (d) => d instanceof Date;
export const isEmpty = (o) => Object.keys(o).length === 0;
export const isObject = (o) => o != null && typeof o === "object";
export const properObject = (o) => (isObject(o) && !o.hasOwnProperty ? { ...o } : o);
export const isDate = d => d instanceof Date;
export const isEmpty = o => Object.keys(o).length === 0;
export const isObject = o => o != null && typeof o === 'object';
export const hasOwnProperty = (o, ...args) => Object.prototype.hasOwnProperty.call(o, ...args)
export const isEmptyObject = (o) => isObject(o) && isEmpty(o);
25 changes: 1 addition & 24 deletions src/utils/index.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { isDate, isEmpty, isObject, properObject } from './';
import { isDate, isEmpty, isObject } from './';

describe('utils', () => {

Expand Down Expand Up @@ -70,27 +70,4 @@ describe('utils', () => {
expect(isObject(value)).toBe(false);
});
});

describe('.properObject', () => {
it('returns given object when object has keys and hasOwnProperty function', () => {
const o = { a: 1 };
const a = [1];
expect(properObject(o)).toBe(o);
expect(properObject(a)).toBe(a);
});

it('returns given value when value is not an object', () => {
const o = 'hello';
expect(properObject(o)).toBe(o);
});

it('returns object that has given keys and hasOwnProperty function when given object is created from a null', () => {
const o = Object.create(null);
o.a = 1;
const actual = properObject(o);
expect(actual).toEqual({ a: 1 });
expect(typeof actual.hasOwnProperty === 'function').toBe(true);
expect(actual).not.toBe(o);
});
});
});

0 comments on commit b281292

Please sign in to comment.