correctly expose flow typings

[halbgut]

With this PR consumers can use the Flow type definitions without needing any configuration.

[halbgut]

Apparently references don't work in titles... #1232

[coveralls]

Coverage remained the same at 94.686% when pulling 7b31f96 on Kriegslustig:feature/correctly-expose-flow-typings into c6bd40c on mobxjs:master.

[coveralls]

Coverage remained the same at 94.686% when pulling 12c5b2c on Kriegslustig:feature/correctly-expose-flow-typings into c6bd40c on mobxjs:master.

[mweststrate]

@Kriegslustig just double check: you verified this will work when installing the package and import stuff using import { x } from "mobx"?

[halbgut]

Yes, this works. With one caveat though, consumers will need to remove MobX from flow config [libs] section. I will update the README accordingly.

--- a/.flowconfig
+++ b/.flowconfig
@@ -5,7 +5,6 @@
 [include]
 
 [libs]
-./node_modules/mobx/lib/mobx.js.flow
 /flow-typed
 
 [options]

[halbgut]

I documented it in the README. Should I also write something into the changelog?

[coveralls]

Coverage remained the same at 94.686% when pulling 548ce0c on Kriegslustig:feature/correctly-expose-flow-typings into c6bd40c on mobxjs:master.

[mweststrate]

@Kriegslustig yep, please put it in the changelog.md as well :)

[halbgut]

done 👍

[coveralls]

Coverage decreased (-0.04%) to 94.649% when pulling b14f7e9 on Kriegslustig:feature/correctly-expose-flow-typings into c6bd40c on mobxjs:master.

[mweststrate]

Thanks! Merged and will release soon as 3.4.0

[zhaocnus]

@Kriegslustig, @mweststrate:

I think a lot of people might accidentally update MobX from v3.3.3 to v3.4.0 (by just removing and reinstalling all the node_modules). If they do that but they still use MobX's flow type definition based on the old README, they will get a ton of flow errors.

I have a blog post to explain this: https://medium.com/@xiaoyangzhao/mobx-and-flow-typed-1d411a7b47fb

Maybe we need to provide more info in the readme?

[halbgut]

@zhaocnus have you read the changelog? Maybe we can extend this information, if it's not sufficient.

[zhaocnus]

@Kriegslustig Yes I think extending the information will be helpful (especially for people who are not an expert in flow). But it not necessary.

I did check the changelog but I wish I have seen it earlier :)

This is how I encountered the issue:

1. I did a refresh install of all my node modules (removed node_modules folder and package-lock.json, and run npm i). I have "mobx": "^3.2.2", in my package.json. npm i installed mobx 3.4.1.
2. I still have node_modules/mobx/lib/mobx.js.flow in .flowconfig

So this exposes all the mobx type definitions to the global scope, and flow throws a lot of errors. It took me a while to figure the issue. I hope I could have checked the README and changelog earlier.

So I figure adding some more info might save other people's time later.

[mweststrate]

PR for changelog is welcome! (and tip: make sure that removing node_modules doesn't upgrade your modules unintentionally. Use yarn, npm lock files or npm shrinkwrap, without them it is impossible to create reliable / reproducible builds for your project) Op vr 29 dec. 2017 om 09:56 schreef Luca Nils Schmid < [email protected]>:

…

@zhaocnus <https://github.com/zhaocnus> have you read the changelog <https://github.com/mobxjs/mobx/blob/master/CHANGELOG.md#340>? Maybe we can extend this information, if it's not sufficient. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1254 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABvGhMLUX_yzW1wsN08RPlkDYi7EsSd_ks5tFKk-gaJpZM4Qqjbs> .

[zhaocnus]

@mweststrate Thanks for the tip! I've learned my lesson. I will create a PR.