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

[General] [Fixed] Flow errors from YellowBox and BubblingEventHandler #28520

Merged
merged 1 commit into from
Apr 7, 2020

Conversation

thymikee
Copy link
Contributor

@thymikee thymikee commented Apr 3, 2020

Summary

Fixes Flow errors that surfaced with 0.62.1 release.

Targeting 0.62-stable branch, because this is an effect of applying patches that were relying on prior changes:

  • YellowBox is now removed from from master
  • BubblingEventHandler was removed in 0676ebf which partially built on top of bd2b7d6, later reverted in 27a3248

cc @alloy @kelset

Changelog

[General] [Fixed] Flow errors from YellowBox and BubblingEventHandler

Test Plan

Flow check passes

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Callstack Partner: Callstack Partner labels Apr 3, 2020
@thymikee thymikee requested a review from alloy April 3, 2020 20:53
@pull-bot
Copy link

pull-bot commented Apr 3, 2020

Warnings
⚠️

❔ Base Branch - The base branch for this PR is something other than master. Are you sure you want to merge these changes into a stable release? If you are interested in backporting updates to an older release, the suggested approach is to land those changes on master first and then cherry-pick the commits into the branch for that release. The Releases Guide has more information.

Generated by 🚫 dangerJS against a049130

@kelset
Copy link
Contributor

kelset commented Apr 6, 2020

hey Mike, thanks for the PR! Weird that this flow error wasn't picked up 🤔 It's probably because now the template is not flow-typed I'd guess

@thymikee
Copy link
Contributor Author

thymikee commented Apr 6, 2020

This is unrelated to the template. The flow check failed when run from the root of the repo as well.

Copy link
Contributor

@alloy alloy left a comment

Choose a reason for hiding this comment

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

Makes sense, thanks 👍

Related; do we run flow check on CI?

@thymikee
Copy link
Contributor Author

thymikee commented Apr 6, 2020

The analyze_code and test_js through run-ci-javascript-tests.js file both run flow check.

@alloy
Copy link
Contributor

alloy commented Apr 7, 2020

I was waiting to merge this so we could cherry-pick fixes in chronological order, but I realise now that it will be easier to fix merge conflicts when cherry-picking than on this PR; so merging now.

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. p: Callstack Partner: Callstack Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants