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 Flow issues introduced in #7716 #7735

Closed
zpao opened this issue Sep 14, 2016 · 5 comments
Closed

Fix Flow issues introduced in #7716 #7735

zpao opened this issue Sep 14, 2016 · 5 comments
Assignees

Comments

@zpao
Copy link
Member

zpao commented Sep 14, 2016

We missed this because Flow wasn't being run properly on Travis but now that it is, it's clear that #7716 introduced a bunch of new Flow failures. Most of these shouldn't be too bad and I like to say "you break it you buy it" so this one is for you @aweary :)

There were a couple other pre-existing Flow issues that @vjeux is going to fix, so mostly this should just be the TestRenderer issues.

@aweary
Copy link
Contributor

aweary commented Sep 14, 2016

Was it just an issue with Flow on Travis? I didn't actually get any errors for flow locally in Nuclide, maybe something is wrong on my end there :)

I will fix this 👍

@vjeux
Copy link
Contributor

vjeux commented Sep 14, 2016

The first three were introduced by #7649

src/renderers/shared/stack/reconciler/ReactOwner.js:76
 76:     transaction,
         ^^^^^^^^^^^ parameter `transaction`. Missing annotation

src/renderers/testing/ReactTestMount.js:24
 24:   createNodeMock: (element: ReactElement) => Object,
                                 ^^^^^^^^^^^^ ReactElement. Application of polymorphic type needs <list of 1 argument>. (Can use `*` for inferrable ones)

src/renderers/testing/ReactTestReconcileTransaction.js:60
 60: function ReactTestReconcileTransaction(testOptions) {
                                            ^^^^^^^^^^^ parameter `testOptions`. Missing annotation

The 17 others were introduced by #7716

So looks like all of them are yours @aweary :)

Let me know if you need any help fixing them, I'd be happy to.

@vjeux
Copy link
Contributor

vjeux commented Sep 14, 2016

@aweary the issue was just on travis, running flow locally should show you all those errors.

@zpao
Copy link
Member Author

zpao commented Sep 14, 2016

You should be seeing the issue locally in Nuclide (I am right now), but it's possible they aren't there due to a version mismatch. Not sure how the integration works but maybe your global flow isn't the same version we're using in the flowconfig/package.json.

@aweary
Copy link
Contributor

aweary commented Sep 14, 2016

@zpao it looks like there is a version mismatch, I've got 0.29.0 globally. Either way, I'll make sure to run flow in the terminal as well for future PRs 👍

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

No branches or pull requests

3 participants