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

Fix toOrderedMap, toOrderedSet Flow annotations #850

Closed
wants to merge 1 commit into from

Conversation

rvikmanis
Copy link
Contributor

No description provided.

@facebook-github-bot
Copy link

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!

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@marudor
Copy link
Contributor

marudor commented Apr 21, 2016

Actually this will introduce some problems.
Thats why I left them as Map/Set for now.
But the main signatures for Maps/Set is the same for Ordered.
So it shouldn't be a problem to leave them.

@mitermayer
Copy link
Contributor

I also hit on this problem

@marudor on the project https://github.com/facebook/draft-js with immutable '3.7.4' the flow build works just fine.

But since the dependency on the package.json is set as '^0.7.4' the version that gets installed via npm install is ''3.8.1" which is incompatible and fails

@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

I fixed the merge conflicts in this PR / added tests / updated annotations for OrderedSet and OrderedMap in this PR: #1027

@kozlitinaelja
Copy link
Contributor

Hi there. I am starting handling backlog. I know it is been a while since you've submitted PR, sorry for such delay. The are some conflicts. Could you, please, rebase? 😅

@howardjing
Copy link
Contributor

@kozlitinaelja #1027 is merged into master, which is a superset of this PR. I think this PR can be closed.

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

8 participants