-
Notifications
You must be signed in to change notification settings - Fork 842
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
Downgraded lodash 3.10.1 (to align with Kibana) #359
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
export const always = () => true; | ||
|
||
export const never = () => false; | ||
|
||
export const isUndefined = (value) => { | ||
return value === undefined; | ||
}; | ||
|
||
export const isNull = (value) => { | ||
return value === null; | ||
}; | ||
|
||
export const isNil = (value) => { | ||
return isUndefined(value) || isNull(value); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
import { always, never, isNil, isUndefined, isNull } from './common_predicates'; | ||
|
||
describe('common predicates', () => { | ||
|
||
test('always', () => { | ||
[ undefined, null, 'a', 1, true, false, Date.now(), {}, [], /.*/ ].forEach(value => { | ||
expect(always(value)).toBe(true); | ||
}); | ||
}); | ||
|
||
test('never', () => { | ||
[ undefined, null, 'a', 1, true, false, Date.now(), {}, [], /.*/ ].forEach(value => { | ||
expect(never(value)).toBe(false); | ||
}); | ||
}); | ||
|
||
test('isUndefined', () => { | ||
[ undefined ].forEach(value => { | ||
expect(isUndefined(value)).toBe(true); | ||
}); | ||
[ null, 'a', 1, true, false, Date.now(), {}, [], /.*/ ].forEach(value => { | ||
expect(isUndefined(value)).toBe(false); | ||
}); | ||
}); | ||
|
||
test('isNull', () => { | ||
[ null ].forEach(value => { | ||
expect(isNull(value)).toBe(true); | ||
}); | ||
[ undefined, 'a', 1, true, false, Date.now(), {}, [], /.*/ ].forEach(value => { | ||
expect(isNull(value)).toBe(false); | ||
}); | ||
}); | ||
|
||
test('isNil', () => { | ||
[ undefined, null ].forEach(value => { | ||
expect(isNil(value)).toBe(true); | ||
}); | ||
[ 'a', 1, true, false, Date.now(), {}, [], /.*/ ].forEach(value => { | ||
expect(isNil(value)).toBe(false); | ||
}); | ||
}); | ||
|
||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
export * from './common_predicates'; | ||
export * from './lodash_predicates'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
export { | ||
isFunction, | ||
isArray, | ||
isString, | ||
isBoolean, | ||
isDate, | ||
isNumber, | ||
isNaN, | ||
} from 'lodash'; | ||
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. Why not remove lodash entirely? Can we internalize these functions? 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. we should... but it'll have to wait to a different pr... (there's more to this when it comes to removing lodash... we'll need to remove other usages as well - e.g. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
import { isNil } from 'lodash'; | ||
import { isNil } from './predicate'; | ||
|
||
const defaultRand = Math.random; | ||
|
||
|
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.
Can we export the predicates from the root modules,
services
?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 can... thought about it... decided not to... but I can still do that.
Why decided not to? With generic services it somewhat feels off to export ALL possible utility functions under one module. the services sub-modules really don't have anything in common aside from the fact that they're all a bunch of utilities. To me it feels more appropriate to refer to the specific module when importing utilities.
But... that's my opinion... if you insist I can still export all to the higher services module
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 you're right. In fact we should probably apply this pattern to the rest of the submodules under
services
.