redux-utilities / redux-promise

FSA-compliant promise middleware for Redux.
MIT License
2.67k stars 134 forks source link

chore: updates all devtools to use the even newer hotness #56

Closed JaKXz closed 6 years ago

JaKXz commented 6 years ago

I just used the name of the test folder that was there previously, but just moved it out of src so it wouldn't shipped in the build step. Since it follows the jest-files naming convention I think it's fine.

Using babel 7 was just because the old preset-env did not have the object spread operator included, which I think is just out of date at this point so I thought we should move forward.

timche commented 6 years ago

I just used the name of the test folder that was there previously, but just moved it out of src so it wouldn't shipped in the build step. Since it follows the jest-files naming convention I think it's fine.

Personally I'd go with consistency as we have it in our other repos as test/*.test.js

Using babel 7 was just because the old preset-env did not have the object spread operator included, which I think is just out of date at this point so I thought we should move forward.

Got it. Nevertheless we need to keep an eye when Babel 7 has been fully released and Jest is supporting it then too, so we don't have to maintain two versions.

codecov[bot] commented 6 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@ae01607). Click here to learn what that means. The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##             master    #56   +/-   ##
=======================================
  Coverage          ?   100%           
=======================================
  Files             ?      1           
  Lines             ?      7           
  Branches          ?      3           
=======================================
  Hits              ?      7           
  Misses            ?      0           
  Partials          ?      0
Impacted Files Coverage Δ
src/index.js 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ae01607...d0a638d. Read the comment docs.

timche commented 6 years ago

There are some promise errors on test:

$ yarn test
yarn run v1.3.2
$ jest
(node:3139) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'type' of undefined
(node:3139) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 5)
(node:3139) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
JaKXz commented 6 years ago

@timche I have fixed the ones that I could find (by adding .catch(noop) to the Promise.reject(err) calls in the tests), but those are the ones I could not find the source of... 😕

I have a feeling it might be an upstream issue because we use the .rejects assertion, which might not be catching the thrown error in the async function? I'm really not sure though.

timche commented 6 years ago

Try wrapping your async function contents in a try/catch block and console.log.

try {
  await // ...
} catch (e) {
  console.log(e)
}
JaKXz commented 6 years ago

@timche fixed :) I think this is ready to go. I think we should publish v0.6.0 when this is merged and then I can take a look at triaging the other open PRs/issues.