-
-
Notifications
You must be signed in to change notification settings - Fork 15.2k
Replace lodash utils #131
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
Merged
Merged
Replace lodash utils #131
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
3aef469
Add replacement utils for lodash modules + tests
frankychung 9724f2c
Replace lodash module imports with utils
frankychung a5a6bf4
Remove lodash from package.json
frankychung ddbfc5a
Delint isPlainObject util
frankychung d76b216
Improve identity util test to check reference
frankychung 974b7b8
Check null/undefined case for isPlainObject
frankychung File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
export default function identity(value) { | ||
return value; | ||
} | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
export default function isPlainObject(obj) { | ||
return typeof obj === 'object' && Object.getPrototypeOf(obj) === Object.prototype; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This one does not work. isPlainObject(null) // TypeError: Object.getPrototypeOf called on non-object There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you, added another test case for calling with |
||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
export default function pick(obj, fn) { | ||
return Object.keys(obj).reduce((result, key) => { | ||
if (fn(obj[key])) { | ||
result[key] = obj[key]; | ||
} | ||
return result; | ||
}, {}); | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
import expect from 'expect'; | ||
import identity from '../../src/utils/identity'; | ||
|
||
describe('Utils', () => { | ||
describe('identity', () => { | ||
it('should return first argument passed to it', () => { | ||
var test = { 'a': 1 }; | ||
expect(identity(test, 'test')).toEqual(test); | ||
}); | ||
}); | ||
}); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
import expect from 'expect'; | ||
import isPlainObject from '../../src/utils/isPlainObject'; | ||
|
||
describe('Utils', () => { | ||
describe('isPlainObject', () => { | ||
it('should return true only if plain object', () => { | ||
function Test() { | ||
this.prop = 1; | ||
} | ||
|
||
expect(isPlainObject(new Test())).toBe(false); | ||
expect(isPlainObject(new Date())).toBe(false); | ||
expect(isPlainObject([1, 2, 3])).toBe(false); | ||
expect(isPlainObject({ 'x': 1, 'y': 2 })).toBe(true); | ||
}); | ||
}); | ||
}); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
import expect from 'expect'; | ||
import pick from '../../src/utils/pick'; | ||
|
||
describe('Utils', () => { | ||
describe('pick', () => { | ||
it('should return object with picked values', () => { | ||
const test = { 'name': 'lily', 'age': 20 }; | ||
expect(pick(test, x => typeof x === 'string')).toEqual({ 'name': 'lily' }); | ||
}); | ||
}); | ||
}); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually lodash's identity function is the same as yours but yes this does make sense to totally remove lodash as dependancy if this will be only one function needed from it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's probably best to put these all in a single file and export them individually that way you avoid the overhead of including each file, I think it adds ~3 lines and a bunch of bytes for every file you include.
This also has the benefit of keeping all the utils together.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As an aside: identity would then be something like:
export const identity = x => x;
which is just 😎There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@goatslacker Grouping these in one file sounds like a good idea to me however I worry it doesn't follow the style of the rest of utils of exporting one function per module (or as @taylorhakes mentions inlining some of them, especially simple ones like identity). Also, exporting arrow functions is pleasantly succinct but also another question of style :). @gaearon thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As @goatslacker suggested, using an arrow function would be better because:
identity
is notnew
-ableidentity
has noprototype
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
I think it's just a matter whether we want to have it as a named function or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not the only implication of arrow functions 😉