Skip to content
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

Updated flow types #881

Closed
wants to merge 1 commit into from
Closed

Updated flow types #881

wants to merge 1 commit into from

Conversation

johanneslumpe
Copy link

toOrderedSet and toOrderedMap were updated in the typescript definitions, but not the flow ones.

`toOrderedSet` and `toOrderedMap` [were updated in the typescript definitions](#761), but not the flow ones.
@ghost ghost added the CLA Signed label May 22, 2016
@johanneslumpe
Copy link
Author

johanneslumpe commented May 22, 2016

@leebyron The changes in the above referenced PR caused the flow checks for draft-js to break. The changes here fix some of these. But there are more issues cropping up which have to do with incompatible types. Locally I tried copying over all the methods (minus the update methods) from the Map to the OrderedMap type and update the return values. Is this the correct way to deal with this?

If so, I can update this PR to include those changes as well!

@hellendag
Copy link

hellendag commented Jun 20, 2016

I'm going to accept the PR to switch Draft to ~3.7.4 to sidestep this. Would be awesome to be able to switch it back soon.

@ghost ghost added the CLA Signed label Jul 12, 2016
@wokalski
Copy link
Contributor

@johanneslumpe This repo is not actively maintained as far as I can see. I created a PR in flow-typed to add flow definitions for immutable there.

My fork is here. Feel free to submit a PR with your changes there.

@lacker
Copy link

lacker commented Dec 6, 2016

So #878 refactored the flow types and also added tests for them. Would you mind updating this PR to merge those changes in, and also add a test that catches this? Thanks!

@howardjing
Copy link
Contributor

@lacker I just submitted a PR that address this issue along with OrderedMap / OrderedSet typing issues: #1027

@lacker
Copy link

lacker commented Dec 21, 2016

OK, I am going to close this one in favor of 1027.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants