-
-
Notifications
You must be signed in to change notification settings - Fork 380
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
On client-side init error return rejected promise rather than throw #87
Conversation
Codecov Report
@@ Coverage Diff @@
## master #87 +/- ##
======================================
+ Coverage 88.5% 90.5% +2%
======================================
Files 15 15
Lines 200 200
Branches 54 54
======================================
+ Hits 177 181 +4
+ Misses 21 17 -4
Partials 2 2
Continue to review full report at Codecov.
|
Thanks for this improvement, but if you use |
Ah! Updated (was up working much too late). |
aef0ce6
to
c351cff
Compare
I've updated tests to cover each error branch but I can't figure out how to stub |
src/loadComponents.js
Outdated
}" is not found, client and server modules are not sync. You are probably not using the same resolver on server and client.` | ||
|
||
console.error(errorMessage) | ||
return Promise.reject(errorMessage) |
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 we should just do return Promise.reject(new Error(...))
, it should be sufficient. So :
- Please remove log
- Please call
Promise.reject
with an error
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.
return Promise.reject(new Error(...))
is much cleaner 👍
@damassi don't worry about test coverage it is not a problem. |
11ab531
to
0bc00fc
Compare
@damassi tests are broken :/ |
@neoziro - looks like a CI thing -- let me kick off another commit... |
thanks! |
This swaps out an error with a rejected promise so that client UI doesn't break. Noticed when used in conjunction with
@artsy/express-reloadable
that when reloads occur and module ids increment an error occurs when really there's no problem.(Really excellent lib -- was totally painless to setup compared to all of the others 👍)