Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions src/utils/shallowEqual.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,6 @@ export default function shallowEqual(objA, objB) {

return false;
}

var valA = objA[keysA[i]];
var valB = objB[keysA[i]];

if (valA !== valB) {
return false;
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is dead code. This is already present in the if statement.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep.


return true;
Expand Down
134 changes: 134 additions & 0 deletions test/utils/shallowEquality.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
import expect from 'expect';
import shallowEqualScalar from '../../src/utils/shallowEqualScalar';
import shallowEqual from '../../src/utils/shallowEqual';

describe('Utils', () => {
// More info: https://github.com/gaearon/redux/pull/75#issuecomment-111635748
describe('shallowEqualScalar', () => {
it('returns true if both arguments are the same object', () => {
const o = { a: 1, b: 2 };
expect(shallowEqualScalar(o, o)).toBe(true);
});

it('returns false if either argument is null', () => {
expect(shallowEqualScalar(null, {})).toBe(false);
expect(shallowEqualScalar({}, null)).toBe(false);
});

Copy link
Contributor

Choose a reason for hiding this comment

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

There's a lot of tests for the arrays, but I'd expect to see more tests with objects.
For some inspiration: https://github.com/facebook/react/blob/master/src/shared/utils/__tests__/shallowEqual-test.js

it('returns true if arguments fields are equal', () => {
expect(
shallowEqualScalar(
{ a: 1, b: 2, c: undefined },
{ a: 1, b: 2, c: undefined }
)
).toBe(true);

expect(
shallowEqualScalar(
{ a: 1, b: 2, c: 3 },
{ a: 1, b: 2, c: 3 }
)
).toBe(true);
});

it('returns false if first argument has too many keys', () => {
expect(
shallowEqualScalar(
{ a: 1, b: 2, c: 3 },
{ a: 1, b: 2 }
)
).toBe(false);
});

it('returns false if second argument has too many keys', () => {
expect(
shallowEqualScalar(
{ a: 1, b: 2 },
{ a: 1, b: 2, c: 3 }
)
).toBe(false);
});

it('returns false if arguments have keys dont have same value', () => {
expect(
shallowEqualScalar(
{ a: 1, b: 2 },
{ a: 1, b: 3 }
)
).toBe(false);
});

it('returns false if arguments have field that are objects', () => {
const o = {};
expect(
shallowEqualScalar(
{ a: 1, b: 2, c: o },
{ a: 1, b: 2, c: o }
)
).toBe(false);
});

it('returns false if arguments have different keys', () => {
expect(
shallowEqualScalar(
{ a: 1, b: 2, c: undefined },
{ a: 1, bb: 2, c: undefined }
)
).toBe(false);
});
});

describe('shallowEqual', () => {
it('returns true if arguments fields are equal', () => {
expect(
shallowEqual(
{ a: 1, b: 2, c: undefined },
{ a: 1, b: 2, c: undefined }
)
).toBe(true);

expect(
shallowEqual(
{ a: 1, b: 2, c: 3 },
{ a: 1, b: 2, c: 3 }
)
).toBe(true);

const o = {};
expect(
shallowEqual(
{ a: 1, b: 2, c: o },
{ a: 1, b: 2, c: o }
)
).toBe(true);
});

it('returns false if first argument has too many keys', () => {
expect(
shallowEqual(
{ a: 1, b: 2, c: 3 },
{ a: 1, b: 2 }
)
).toBe(false);
});

it('returns false if second argument has too many keys', () => {
expect(
shallowEqual(
{ a: 1, b: 2 },
{ a: 1, b: 2, c: 3 }
)
).toBe(false);
});

it('returns false if arguments have different keys', () => {
expect(
shallowEqual(
{ a: 1, b: 2, c: undefined },
{ a: 1, bb: 2, c: undefined }
)
).toBe(false);
});
});

});