Add Default Export to Flow Type definitions #961
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks! |
|
Just signed the contributors license agreement |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
|
I also want to mention that it would be great if we could somehow get some strong typing behind Map and List literals. For example, If I have something like |
|
Does this PR still make sense now that we merged #878 ? |
|
@ccorcos, is the issue addressed in this PR still exists? If no, please, close. 😅 |
|
@kozlitinaelja yes -- the definitions are still declared globally. Therefore, you can only have one Map which refers to the immutable Map and not window.Map. https://flowtype.org/docs/declarations.html#declaration-files |
|
@ccorcos could you recommend a way to test this change that would otherwise currently fail? I just added a flow type test that ensures the native es6 collections can be used alongside the Immutable.js ones. |
|
Are you doing |
The flow type definition tests added in #878 relied on declaring a module, which required .flowconfig to explicitly include the flow definition files, and made exports unclear. This reverts some of the structural changes to the immutable.js.flow from that PR and simplifies the .flowconfig test file to ensure tests are against the *local* copy of immutable. This uncovered a few issues with tests including a mistaken sense that types were exportes as classes and that Set intersections were causing issues. Fixes #961
The flow type definition tests added in #878 relied on declaring a module, which required .flowconfig to explicitly include the flow definition files, and made exports unclear. This reverts some of the structural changes to the immutable.js.flow from that PR and simplifies the .flowconfig test file to ensure tests are against the *local* copy of immutable. This uncovered a few issues with tests including a mistaken sense that types were exportes as classes and that Set intersections were causing issues. Fixes #961
|
In case anyone else lands here. Doing...
is a workaround until there's a release that resolves the issue. That got rid of the flow errors for me. |
This PR solves this issue:
#854
In the Flow documentation the show an example where you define library definitions in the
libsoption. So you may be inclined to use these type declarations by having the following in your flowconfig:The problem with this approach is that those type definitions are now global. Anytime you define a Map type, its going to defer to the Immutable Map and ever the window.Map type definition.
In a separate part of the documentation the talk about delcaration files using the
.js.flowapproach. Now instead of adding the files as a global declaration in libs, you can simplyimport immutable from 'immutable'and everything just works :)