Transpile for non-flow users, and add tests

expo / redux-effex

Spin off async functions to perform side effects

109 stars 2 forks source link

Transpile for non-flow users, and add tests #3

Closed jamespic closed 7 years ago

jamespic commented 8 years ago

Hi,

There are a couple of changes that we needed to make to make this usable on our project:

The version that's published to NPM currently includes Flow annotations, as well as a few other ES-next features. I added Babel transpilation (preserving Flow annotations) to the publish step, to allow non-Flow users (and users with different Babel/webpack configs) to use redux-effex from NPM.
I added a few unit tests.

There are no changes to index.js, but I moved it to src, to simplify transpilation.

pbomb commented 8 years ago

I want to make sure that I can use this branch in my code that expects the flow types to exist. I also would probably prefer to use Jest instead of Mocha (which doesn't change much). @brentvatne, do you have an opinion on testing framework? I'm very much in favor of adding tests to this library.

jamespic commented 8 years ago

I deliberately used transform-flow-comments, rather than stripping the flow types out, so they should still be there (although I don't use Flow, so I can't be sure it all still works as intended - you could test it with the prebuilt version on my dist branch).

I mostly went with Mocha out of familiarity (although I generally choose it on projects for its light weight), but as you say, it's an easy change if there's a project preference for Jest.

pbomb commented 7 years ago

Ah, thanks, @jamespic. I just figured it was using the transform-flow-strip-types plugin. It might be better to separate the exported flow types into an index.js.flow file and then we can strip the types through babel. That should work for Flow consumers and make index.js a bit smaller. I'm fine to take this approach to start given that it's already done.

brentvatne commented 7 years ago

I prefer Jest but I'm totally OK with Mocha for now also, better some tests than none, thanks! Also agree re: @pbomb's comments above re: exporting from an index.js.flow file.

brentvatne commented 7 years ago

@pbomb - I changed it over to use index.js.flow and pushed version 1.1.1

pbomb commented 7 years ago

Sounds good. I'll see about converting the tests to Jest.