-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Refactor assertString: Faster, less nested and more consistent. #2372
base: master
Are you sure you want to change the base?
Changes from 3 commits
a8f5eb7
241c4a8
69cb5fa
f726e2d
f8e62de
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 |
---|---|---|
@@ -1,11 +1,4 @@ | ||
export default function assertString(input) { | ||
const isString = typeof input === 'string' || input instanceof String; | ||
|
||
if (!isString) { | ||
let invalidType = typeof input; | ||
if (input === null) invalidType = 'null'; | ||
else if (invalidType === 'object') invalidType = input.constructor.name; | ||
|
||
throw new TypeError(`Expected a string but received a ${invalidType}`); | ||
} | ||
if (input === undefined) throw new TypeError(`Expected a string but received a ${input}`); | ||
if (input.constructor.name !== 'String') throw new TypeError(`Expected a string but received a ${input.constructor.name}`); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,8 @@ | |
*/ | ||
import assert from 'assert'; | ||
import typeOf from '../src/lib/util/typeOf'; | ||
import assertString from '../src/lib/util/assertString'; | ||
|
||
|
||
describe('Util', () => { | ||
it('should validate different typeOf', () => { | ||
|
@@ -18,3 +20,29 @@ describe('Util', () => { | |
assert.notStrictEqual(typeOf([]), 'object'); | ||
}); | ||
}); | ||
|
||
describe('assertString', () => { | ||
it('Should throw an error if no argument is provided', () => { | ||
assert.throws(() => { assertString(); }, TypeError); | ||
}); | ||
|
||
it('Should throw an error if the argument is not a string, number', () => { | ||
assert.throws(() => { assertString(123); }, TypeError); | ||
}); | ||
|
||
it('Should throw an error if the argument is not a string, Object', () => { | ||
assert.throws(() => { assertString({}); }, TypeError); | ||
}); | ||
|
||
it('Should throw an error if the argument is not a string, Array', () => { | ||
assert.throws(() => { assertString([]); }, TypeError); | ||
}); | ||
|
||
it('Should not throw an error if the argument is a string', () => { | ||
assert.doesNotThrow(() => { assertString(''); }); | ||
}); | ||
|
||
it('Should not throw an error if the argument is a string', () => { | ||
assert.doesNotThrow(() => { assertString('testing'); }); | ||
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. Can we keep the empty string unit test? Then we will not forget that we accept that on purpose 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. Yes, we can. I'll create and commit it. 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. 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. while we are at it, we should likely also test cases for Maybe 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. I created the
|
||
}); | ||
}); |
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.
should probably check for
null
as well, see #2372 (comment)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 will add null validation and create tests for both cases.