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

Add support for NaN value #49

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 33 additions & 5 deletions index.d.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,37 @@
export function diff (originalObj: object, updatedObj: object): object
type DeepPartial<T> = {
[K in keyof T]?: DeepPartial<T[K]>
}

export function addedDiff (originalObj: object, updatedObj: object): object
interface IDictionary<T> {
[key: string]: T;
}

export function deletedDiff (originalObj: object, updatedObj: object): object
/*
In "deep-object-diff" there're 2 scenarios for a property diff:
1. If the property is an object or primitive, the diff is a deep partial;
2. If the property is an array, the diff is a dictionary.
Its keys are indices, and the values are deep partials of the change.
*/
type PropertyDiff<T> = T extends Array<infer Elem>
? IDictionary<Elem>
: DeepPartial<T>;

export function updatedDiff (originalObj: object, updatedObj: object): object
export type Diff<T> = {
[P in keyof T]?: PropertyDiff<T[P]>;
};

export function detailedDiff (originalObj: object, updatedObj: object): object
export interface IDetailedDiff<T> {
added: Diff<T>;
deleted: Diff<T>;
updated: Diff<T>;
}

export function diff<T> (originalObj: T, updatedObj: T): Diff<T>

export function addedDiff<T> (originalObj: T, updatedObj: T): Diff<T>

export function deletedDiff<T> (originalObj: T, updatedObj: T): Diff<T>

export function updatedDiff<T> (originalObj: T, updatedObj: T): Diff<T>

export function detailedDiff<T> (originalObj: T, updatedObj: T): IDetailedDiff<T>
4 changes: 4 additions & 0 deletions src/diff/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ import { isDate, isEmpty, isObject, properObject } from '../utils';
const diff = (lhs, rhs) => {
if (lhs === rhs) return {}; // equal return no diff

if (typeof lhs === "number" && typeof rhs === "number") {
if (isNaN(lhs) && isNaN(rhs)) return {}; // NaN is equal
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could use Number.isNaN to check both Number type and NaN value simultaneously.

-   if (typeof lhs === "number" && typeof rhs === "number") {
-     if (isNaN(lhs) && isNaN(rhs)) return {}; // NaN is equal
-  }
+   if (Number.isNan(lhs) && Number.isNan(rhs)) return {}; // NaN is equal

Copy link

Choose a reason for hiding this comment

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

@anko you meant to cap the last 'n' --> Number.isNaN


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

const l = properObject(lhs);
Expand Down
17 changes: 14 additions & 3 deletions src/diff/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ describe('.diff', () => {
['object', { a: 1 }],
['array', [1]],
['function', () => ({})],
['NaN', NaN],
['date', new Date()],
['date with milliseconds', new Date('2017-01-01T00:00:00.637Z')],
])('returns empty object when given values of type %s are equal', (type, value) => {
Expand Down Expand Up @@ -56,7 +57,7 @@ describe('.diff', () => {
});

test('returns subset of right hand side value when nested values differ', () => {
expect(diff({ a: { b: 1, c: 2} }, { a: { b: 1, c: 3 } })).toEqual({ a: { c: 3 } });
expect(diff({ a: { b: 1, c: 2 } }, { a: { b: 1, c: 3 } })).toEqual({ a: { c: 3 } });
});

test('returns subset of right hand side value when nested values differ at multiple paths', () => {
Expand All @@ -72,7 +73,7 @@ describe('.diff', () => {
});

test('returns keys as undefined when deleted from right hand side', () => {
expect(diff({ a: 1, b: { c: 2 }}, { a: 1 })).toEqual({ b: undefined });
expect(diff({ a: 1, b: { c: 2 } }, { a: 1 })).toEqual({ b: undefined });
});
});

Expand Down Expand Up @@ -139,7 +140,7 @@ describe('.diff', () => {

test('returns subset of right hand side value when nested values differ', () => {
const lhs = Object.create(null);
lhs.a = { b: 1, c: 2};
lhs.a = { b: 1, c: 2 };
const rhs = Object.create(null);
rhs.a = { b: 1, c: 3 };
expect(diff(lhs, rhs)).toEqual({ a: { c: 3 } });
Expand Down Expand Up @@ -173,5 +174,15 @@ describe('.diff', () => {
expect(diff(lhs, rhs)).toEqual({ b: 2 });
});
});

describe('nested NaN', () => {
test('returns empty object when there is nested NaN value', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This test description confused me.

More concrete wording:

Suggested change
test('returns empty object when there is nested NaN value', () => {
test('treats NaN -> NaN as unchanged', () => {

expect(diff({ a: 1, b: NaN }, { a: 1, b: NaN })).toEqual({});
});

test('returns subset of right hand side when a left hand side value is not a NaN', () => {
expect(diff({ a: 1, b: 2 }, { a: 1, b: NaN })).toEqual({ b: NaN });
});
});
});
});