Never reject with undefined #85

Merged

benjamingr merged 2 commits from patch-1 into master

2016-02-02 08:50:26 +00:00

benjamingr commented

2016-02-01 08:35:00 +00:00

(Migrated from github.com)

That's like throwing undefined, there is no context and it's impossible for users to be aware of what happened.

Even doing something simple like rejecting with an Error helps here.

I'm not sure what's the point of even rejecting here semantically (vs. not fulfilling) - it doesn't make a lot of sense and introduces abort and control at a distance semantics which are highly discouraged when using promises.

Cheers and thanks for the library.

That's like `throw`ing undefined, there is no context and it's impossible for users to be aware of what happened. Even doing something simple like rejecting with an `Error` helps here. I'm not sure what's the point of even rejecting here semantically (vs. not fulfilling) - it doesn't make a lot of sense and introduces abort and control at a distance semantics which are highly discouraged when using promises. Cheers and thanks for the library.

michaelcontento commented

2016-02-02 08:34:37 +00:00

(Migrated from github.com)

Thank you for this PR! Would it be possible for you to fix the CI? As I'd really like to merge this 😃

Thank you for this PR! Would it be possible for you to [fix the CI](https://travis-ci.org/michaelcontento/redux-storage/jobs/106162191#L680)? As I'd really like to merge this :smiley:

benjamingr commented

2016-02-02 08:40:13 +00:00

(Migrated from github.com)

Yup, sorry. Should have looked at the tests.