return promise #11
return promise #11
Conversation
src/zipcelx.js
Outdated
| @@ -17,7 +17,7 @@ export const generateXMLWorksheet = (rows) => { | |||
|
|
|||
| export default (config) => { | |||
| if (!validator(config)) { | |||
| return; | |||
| return undefined; | |||
There was a problem hiding this comment.
This is implicit without return value.
There was a problem hiding this comment.
line 33 will trigger expected no return value consistent-return eslint error if I don't return undefined. I don't have any better idea about this.
There was a problem hiding this comment.
Oh, right. Good point. Considering that this statement is here because a validation failed, I think it would be most suitable to simply throw an error then.
There was a problem hiding this comment.
Throwing an error might not be expected. I think a rejected promise will be better. But it need to change the validator implement. What do you think?
There was a problem hiding this comment.
I think it's hard to find a proper solution without breaking the API. But considering that we are making a could-be breaking change, by suddenly returning from this function, we can just as well throw an error.
In your current implementation, either undefined or Promise is returned. I think it's actually better if either error is thrown or Promise is returned. Then the consumer doesn't have to check for returned value type, but can simply wrap call in catch-try block.
ShallmentMo
force-pushed
the
return-promise
branch
from
404650b
to
818ec3a
Compare
|
@ShallmentMo Thanks. I'll merge in a moment. |
It will be much nicer if we can return a promise, so that we can do some UI stuff after the file is downloaded.