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

Reverting Flow related changes from CI 15749 to get builds back in order #12526

Closed
wants to merge 4 commits into from
Closed

Conversation

hramos
Copy link
Contributor

@hramos hramos commented Feb 22, 2017

Reverting 4 commits that introduced new Flow errors during build process. This has resulted in CI not being able to successfully build. The engineer working on this has been tasked.

63d3ea1
5042bae
6283878
f2687bf

Similar to #12450, but does not revert 118e883.

Unlike #12450, this PR should import successfully as I will be using a newer import tool.

@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Feb 22, 2017
@hramos
Copy link
Contributor Author

hramos commented Feb 22, 2017

Let's see if these three reverts are sufficient to unbreak CI.


type SectionBase<SectionItemT> = {
type SectionBase = {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no-unused-vars: 'SectionBase' is defined but never used.

@hramos
Copy link
Contributor Author

hramos commented Feb 22, 2017

This PR did not initially pass tests:

$ npm run flow -- check

> react-native@1000.0.0 flow /Users/hramos/git/react-native
> flow "check"

Libraries/Experimental/FlatList.js:132
132:   viewabilityConfig?: ViewabilityConfig,
                           ^^^^^^^^^^^^^^^^^ identifier `ViewabilityConfig`. Could not resolve name

I have now reverted f2687bf which resolved the error above.

hramos referenced this pull request Feb 22, 2017
Reviewed By: yungsters

Differential Revision: D4577395

fbshipit-source-id: 9b9099f5bd5f8fe20b5c24eab7e43f298ba665d9
hramos referenced this pull request Feb 22, 2017
Summary:
- Properly inherit flow types from base components, including `defaultProps`
- template-ify the `Item` type as it flows from the `data` prop into `ItemComponent`

Note that for `SectionList` is is harder to do the `Item` typing because each section in the
`sections` array can have a different `Item` type, plus all the optional overrides...not sure how to
tackle that.

Reviewed By: yungsters

Differential Revision: D4557523

fbshipit-source-id: a0c5279fcdaabe6aab5fe11743a99c3715a44032
hramos referenced this pull request Feb 22, 2017
Reviewed By: bvaughn

Differential Revision: D4561262

fbshipit-source-id: 990f338c197851252bea4902fa86249d6e1c975b
hramos referenced this pull request Feb 22, 2017
Reviewed By: bvaughn, yungsters

Differential Revision: D4563798

fbshipit-source-id: 0591cef7c854b525d77e526af783284d9696cb48
@facebook-github-bot facebook-github-bot added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels Feb 23, 2017
@facebook-github-bot
Copy link
Contributor

Something went wrong when importing this pull request. Please cc someone from the team at fb to help with importing this.

@facebook-github-bot facebook-github-bot added Import Failed and removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Feb 23, 2017
@skevy
Copy link
Contributor

skevy commented Feb 23, 2017

@hramos is this needed anymore given the fix that we're attempting to land for Flow in #12533?

@hramos
Copy link
Contributor Author

hramos commented Feb 23, 2017

Not any more, closing.

@hramos hramos closed this Feb 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants