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 Flipper integration on iOS and update it #27426

Closed
wants to merge 1 commit into from

Conversation

charpeni
Copy link
Contributor

@charpeni charpeni commented Dec 5, 2019

Summary

The integration of Flipper on iOS was crashing the template, and test_ios_e2e were failing because of it.

Flipper integration must be wrapped inside FB_SONARKIT_ENABLED.

Flipper's issues with Yoga and YogaKit were solved in Flipper 0.28.0, so let's update our integration to it, and we'll need to use Swift.

We will be able to ship this in 0.62 (cc @grabbou, @axe-fb, @priteshrnandgaonkar), and it should fix test_ios_e2e (cc @hramos) 🤞 .

Changelog

[iOS] [Fixed] - Fix Flipper integration on and update Flipper to 0.30.0

Test Plan

  • Initialized an app with my branch as source.
  • Added [AppDelegate initializeFlipper:application]; to didFinishLaunchingWithOptions.
  • Launched Flipper.
  • Ran the app (debug).

image

  • Ran the app (release).
  • Everything succeeds, no Flipper connection available in release.

@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. Contributor A React Native contributor. labels Dec 5, 2019
@react-native-bot react-native-bot added Platform: iOS iOS applications. Bug labels Dec 5, 2019
@axe-fb
Copy link
Contributor

axe-fb commented Dec 5, 2019

Thanks for working on this. I have imported this, and should be able to land this soon.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@axe-fb has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@axe-fb
Copy link
Contributor

axe-fb commented Dec 5, 2019

Should we separate the version update into a separate PR, just to keep this things clean ?

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@axe-fb has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@charpeni
Copy link
Contributor Author

charpeni commented Dec 5, 2019

I'm not sure if it will work fine with the original Flipper version as I'm on Xcode 11. 😬

@axe-fb
Copy link
Contributor

axe-fb commented Dec 5, 2019

Thats find them. Just imported it, once it is reviewed, we can land it.

@dulmandakh
Copy link
Contributor

why Swift.swift file?

@charpeni
Copy link
Contributor Author

charpeni commented Dec 6, 2019

Flipper requires Swift compiler for YogaKit, and maybe Yoga too as we're using modular header. In order to get it compiled correctly, we need to have at least one Swift file in our project.

Maybe there's a way without this additional Swift file, but I didn't succeed, even with ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @charpeni in 898b1db.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Dec 6, 2019
facebook-github-bot pushed a commit to facebook/flipper that referenced this pull request Dec 10, 2019
Summary:
Sync the Getting Started section about React Native integration with the latest changes in facebook/react-native#27426.

## Changelog

Refresh Getting Started with the latest React Native integration
Pull Request resolved: #676

Test Plan: Tested in RN's template.

Reviewed By: mweststrate

Differential Revision: D18853584

Pulled By: passy

fbshipit-source-id: d001046106743b68d2f08b084c7618d8f768dea1
@friederbluemle
Copy link
Contributor

👋 Thanks for the fixes - Just a few quick questions related to this:

  • How will the now included Swift standard libraries impact binary size (if at all)?
  • If the Swift file is removed from an app project (that does not use Flipper), is the compiler smart enough to also exclude the Swift standard libraries, or are additional settings required?
  • SWIFT_VERSION = 5.0 and SWIFT_OPTIMIZATION_LEVEL = "-Onone" - Are these the defaults? Can it safely be set to something else?
  • Swift.swift is the first and only Swift file in the entire react-native repository (almost 4000 files) - And it sounds like the only reason it's there is to "trick" the compiler. Are there any other potential solutions that achieve the same without the dummy file and embedding the Swift standard libraries?

Sorry for all the questions - Swift newbie here 😇

@dulmandakh
Copy link
Contributor

I have same questions too 😇

@rickhanlonii rickhanlonii mentioned this pull request Dec 18, 2019
30 tasks
@friederbluemle
Copy link
Contributor

@charpeni @axe-fb - I know there has already been some discussion on Discord, but to close the loop it would be great if you could post a quick reply here too.

@dulmandakh
Copy link
Contributor

ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES = YES might add 5mb to an app size

@charpeni
Copy link
Contributor Author

@friederbluemle never be sorry for asking questions!

  • How will the now included Swift standard libraries impact binary size (if at all)?

@dulmandakh is right. We depend on YogaKit, which includes YGLayoutExtensions.swift, and then there's not much we can do for the binary size with Swift, I'm afraid.

But, since Flipper is only running in Debug, we should be able to set ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES to NO in Release. I'll have a look at this.


  • If the Swift file is removed from an app project (that does not use Flipper), is the compiler smart enough also to exclude the Swift standard libraries, or are additional settings required?

Yes, Xcode is smart enough the change all settings related to this. The same thing applies when you add a swift file to a project. I wasn't able to keep that configuration, because the moment I removed the Swift file, Xcode removed everything related to Swift, even if we still have Swift pods. The template folder is not the most pleasant way to deal with that, I'll retry with a sample app and compare the diff with git, it may be more helpful. 😄


  • SWIFT_VERSION = 5.0 and SWIFT_OPTIMIZATION_LEVEL = "-Onone" - Are these the defaults? Can it safely be set to something else?

Yes, it can be safely set to something else. This Podfile script will set the SWIFT_VERSION for YogaKit to 4.1 anyway, but I wouldn't go lower than that.

installer.pods_project.targets.each do |target|
if target.name == 'YogaKit'
target.build_configurations.each do |config|
config.build_settings['SWIFT_VERSION'] = '4.1'
end
end
end


  • Swift.swift is the first and only Swift file in the entire react-native repository (almost 4000 files) - And it sounds like the only reason it's there is to "trick" the compiler. Are there any other potential solutions that achieve the same without the dummy file and embedding the Swift standard libraries?

Well, some say the only reason I did that was to introduce a Swift file into a Facebook repo silently. But this is not completely true.


it sounds like the only reason it's there is to "trick" the compiler.

Exactly, I wasn't able to keep a Swift compiler, or in that case, standard libraries without a Swift file. Weird, right? I can't believe this is not possible, because having a placeholder Swift file seems really odd to me. I didn't look more than a few minutes into that as the goal was to start testing Flipper with React Native and fix tests. As I said earlier, I'll retry that, and use git to compare changes in the project with and without a Swift file. That way, we may be able to trick the compiler without a Swift file.

Thanks for raising those concerns, I'll have a closer look to make sure we release something enjoyable in RN 0.62.

I hope this answers your questions, feel free to reach out again. Otherwise, I'll let you know or cc you into upcoming pull requests.

@rickhanlonii
Copy link
Member

@charpeni have you looked into "ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES to NO in Release"? Seems like that would smooth over the app size increase. As for upgrades, how would that flag impact people who already use swift?

alloy added a commit to alloy/react-native that referenced this pull request Jan 31, 2020
facebook-github-bot pushed a commit that referenced this pull request Feb 3, 2020
Summary:
Related to #27426 & #27565.

The wish to not include an empty Swift file to trigger the correct Xcode settings was voiced in #27426 & #27565.

## Changelog

[iOS] [Fixed] - Remove need for Swift file in the user’s project in order to use Flipper
Pull Request resolved: #27922

Test Plan:
An application created with the template, like so:

```bash
react-native init --template=~/Code/React/react-native TestFlipper
```

…will successfully build, launch, and have Flipper connect.

Differential Revision: D19690592

Pulled By: cpojer

fbshipit-source-id: ee696e0d747d6338534b0c2d62029e64ece02cd3
alloy added a commit to alloy/react-native that referenced this pull request Feb 5, 2020
Summary:
Related to facebook#27426 & facebook#27565.

The wish to not include an empty Swift file to trigger the correct Xcode settings was voiced in facebook#27426 & facebook#27565.

[iOS] [Fixed] - Remove need for Swift file in the user’s project in order to use Flipper
Pull Request resolved: facebook#27922

Test Plan:
An application created with the template, like so:

```bash
react-native init --template=~/Code/React/react-native TestFlipper
```

…will successfully build, launch, and have Flipper connect.

Differential Revision: D19690592

Pulled By: cpojer

fbshipit-source-id: ee696e0d747d6338534b0c2d62029e64ece02cd3
osdnk pushed a commit to osdnk/react-native that referenced this pull request Mar 9, 2020
Summary:
Related to facebook#27426 & facebook#27565.

The wish to not include an empty Swift file to trigger the correct Xcode settings was voiced in facebook#27426 & facebook#27565.

## Changelog

[iOS] [Fixed] - Remove need for Swift file in the user’s project in order to use Flipper
Pull Request resolved: facebook#27922

Test Plan:
An application created with the template, like so:

```bash
react-native init --template=~/Code/React/react-native TestFlipper
```

…will successfully build, launch, and have Flipper connect.

Differential Revision: D19690592

Pulled By: cpojer

fbshipit-source-id: ee696e0d747d6338534b0c2d62029e64ece02cd3
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Sep 1, 2020
We'll actually enable it in the main RN v0.61 -> v0.62 upgrade
commit, if only because running Flipper pre-RN v0.62 isn't
advertised as something you can do. Enabling it means calling the
initialize function, like in facebook/react-native@05f5cb534, but in
the form it took in facebook/react-native@b4d1fcfb2 (and the
conditional that appears there).

This encompasses several RN commits, as follows, several of which
broke builds (which poses a problem even without Flipper enabled).
We lump these in with their fixes, so that we don't knowingly
introduce errors at any commit in our project.

Mostly, these problems can be traced to upgrading the FlipperKit pod
and not handling breaking changes at the same time. Generally,
though, the RN commits aren't very clear or coherent, so I feel
safer flattening all of these even if something could plausibly
stand alone in our project.

----- facebook/react-native@70274f4e9 -----

This commit is unstable. One reason is that `pod install` fails
until `:modular_headers` is added to the `Yoga` pod declaration in
facebook/react-native@898b1db6d. The failure message:

```
[!] The following Swift pods cannot yet be integrated as static
libraries:

The Swift pod `YogaKit` depends upon `Yoga`, which does not define
modules. To opt into those targets generating module maps (which is
necessary to import them from Swift when building as static
libraries), you may set `use_modular_headers!` globally in your
Podfile, or specify `:modular_headers => true` for particular
dependencies.
```

----- facebook/react-native@e8541e03f -----

The claim is that this commit adds running React Dev Tools to the
template. I had thought this was something totally separate from
Flipper (and so this would naturally stand in its own commit), but
no; the added code comes from the `FlipperKitReactPlugin` subspec of
the `FlipperKit` pod, and it'll be invoked in the Flipper
initialization function.

This commit also advances the FlipperKit pod version to v0.23.6; I'm
not aware of any new issues that were specifically introduced in
this version.

Still, seems safest to apply these changes in this giant commit.

----- facebook/react-native@898b1db6d -----

A small handful of changes in `AppDelegate.m`, the Podfile, and
`project.pbxproj` are made here.

The motivation for the `AppDelegate.m` changes isn't clear to me,
unless it's "The integration of Flipper on iOS was crashing the
template"--not super descriptive or helpful in finding the origin of
the issue.

In the Podfile, the addition of `:modular_headers => true` to the
`Yoga` pod declaration fixes a `pod install` failure introduced in
facebook/react-native@70274f4e9, described above.

Also in the Podfile, the change of FlipperKit's version to v0.30.0
introduces a build failure associated with
facebook/flipper@ab121f9d9, with Flipper release tag v0.26.0; see
facebook/react-native#27565 (comment).
That issue was non-optimally resolved in
facebook/react-native@b4d1fcfb2, which we'll take for now in this
commit (see below); a more principled solution is open as
facebook/flipper#1171.

The `project.pbxproj` changes here aren't fully explained, but
here's what I think happened, and what I did about it:

- An empty Swift file was added, to rouse Xcode into being prepared
  to handle Swift files, which is needed for the "YogaKit" dep; see
  facebook/react-native#27426 (comment).
  This step is included in the upgrade guide [1]. The empty Swift
  file was later backed out in facebook/react-native@e3218a0d9 (see
  below), which is why it doesn't appear here.

- The addition of the empty Swift file activated a wizard that
  walked the user through setting up an Objective-C "bridging
  header" file and set some plausibly desired settings in the
  `project.pbxproj` file. RN deleted the bridging header file, but
  the settings remained.

  We had actually already set up a bridging header file a long time
  ago, in d38129b, and this setup wizard didn't activate when I
  created the empty Swift file. I tried to *get* it to activate (to
  better reproduce the settings changes) by deleting our bridging
  header file and re-adding the Swift file, but it didn't work.
  However, the settings that got added in d38129b seem to match
  these settings pretty closely; one notable difference is that a
  SWIFT_VERSION build setting is 4.2 instead of 5.0.

  In any case, the upgrade guide on the Xcode changes [1] only says
  the bridging-header wizard will "most likely" appear, and if it
  doesn't, just keep going.

- They added "English" under `knownRegions` in the project info. We
  have this already, and, in fact, it gets *removed* if I follow an
  automatic deprecation fix that Xcode suggests in the buildtime
  warnings (an instance of zulip#4112). So, ignore this.

----- facebook/react-native@e3218a0d9 -----

This Facebook commit looks fairly coherent, but it's a partial
reversion of facebook/react-native@898b1db6d (just above) with a
tweak on top of it. So, might as well put it into the same commit.

The `project.pbxproj` changes are as follows, as best as I can tell:

- The empty Swift file described just above is removed. This is not
  reflected in the upgrade guide on the Xcode changes [1], which
  still says you need to create the file.

- A `LIBRARY_SEARCH_PATHS` setting is added. This *is* reflected in
  the upgrade guide on the Xcode changes [1]; following those
  instructions does indeed generate a portion of the diff.

- An `LD_RUNPATH_SEARCH_PATHS` setting is added; this is *not*
  reflected in the upgrade guide. It's reportedly crucial; see
  facebook/react-native#27922 (comment).
  I copied the code.

- A `DEAD_CODE_STRIPPING` setting is removed; this is *not*
  reflected in the upgrade guide. Reportedly (and in my experience,
  as best as I can remember and conclude), it's crucial; see
  facebook/react-native#27922 (comment).
  I copied the code.

----- facebook/react-native@8f954b365 -----

Just simple prose adjustments to a comment, but it's substantially
reworked in facebook/react-native@b4d1fcfb2 (which follows).

----- facebook/react-native@b4d1fcfb2 -----

This looks like a partial reversion of
facebook/react-native@898b1db6d (above), with a tweak; the
`FB_SONARKIT_ENABLED` conditional is removed from `AppDelegate.m`,
but that identifier now appears in the `project.pbxproj`.

It also continues the advancement of the FlipperKit pod's version,
this time to v0.30.2, and adjusts some comments.

----- facebook/react-native@f6a8452e7 -----

After an upgrade to FlipperKit v0.32.0 in
facebook/react-native@ada73a354, quickly reverted in
facebook/react-native@4bb17944f, this commit brings us to v0.33.1.

----- facebook/react-native@8858d879e -----

This is the last commit that affects Flipper on iOS before the v0.62
release was made. It doesn't continue the advancement of the
FlipperKit version (and a good thing, too, maybe!). It lists several
transitive dependencies of FlipperKit pods to the Podfile and gives
them each a `:configuration => 'Debug'` setting, and that's it.

It appears to be accounting for a CocoaPods quirk that means that,
before this change is made, all those dependencies would get
included in release builds, increasing the binary size. Might as
well avoid having any commits with that increased size; so, conclude
this giant commit with these changes.

All that's left is to actually enable Flipper, which we'll do in the
main upgrade commit.

[1] react-native-community/upgrade-support#13
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Sep 2, 2020
We'll actually enable it in the main RN v0.61 -> v0.62 upgrade
commit, if only because running Flipper pre-RN v0.62 isn't
advertised as something you can do. Enabling it means calling the
initialize function, like in facebook/react-native@05f5cb534, but in
the form it took in facebook/react-native@b4d1fcfb2 (and the
conditional that appears there).

This encompasses several RN commits, as follows, some of which broke
builds (which poses a problem even without Flipper enabled). We lump
these in with their fixes, so that we don't knowingly introduce
errors at any commit in our project. Some of these problems can be
traced to upgrading the FlipperKit pod and not handling breaking
changes at the same time. Perhaps it wouldn't have been easy for RN
to catch these before merging.

Generally, though, the RN commits aren't very clear or coherent (at
least by our standards), so I feel safer flattening all of these
even if something could plausibly stand alone in our project.

----- facebook/react-native@70274f4e9 -----

This commit is unstable. One reason is that `pod install` fails
until `:modular_headers` is added to the `Yoga` pod declaration in
facebook/react-native@898b1db6d. The failure message:

```
[!] The following Swift pods cannot yet be integrated as static
libraries:

The Swift pod `YogaKit` depends upon `Yoga`, which does not define
modules. To opt into those targets generating module maps (which is
necessary to import them from Swift when building as static
libraries), you may set `use_modular_headers!` globally in your
Podfile, or specify `:modular_headers => true` for particular
dependencies.
```

----- facebook/react-native@e8541e03f -----

The claim is that this commit adds running React Dev Tools to the
template. I had thought this was something totally separate from
Flipper (and so this would naturally stand in its own commit), but
no; the added code comes from the `FlipperKitReactPlugin` subspec of
the `FlipperKit` pod, and it'll be invoked in the Flipper
initialization function.

This commit also advances the FlipperKit pod version to v0.23.6; I'm
not aware of any new issues that were specifically introduced in
this version.

Still, seems safest to apply these changes in this giant commit.

----- facebook/react-native@898b1db6d -----

A small handful of changes in `AppDelegate.m`, the Podfile, and
`project.pbxproj` are made here.

The motivation for the `AppDelegate.m` changes isn't clear to me,
unless it's "The integration of Flipper on iOS was crashing the
template"--not super descriptive or helpful in finding the origin of
the issue.

In the Podfile, the addition of `:modular_headers => true` to the
`Yoga` pod declaration fixes a `pod install` failure introduced in
facebook/react-native@70274f4e9, described above.

Also in the Podfile, the change of FlipperKit's version to v0.30.0
introduces a build failure associated with
facebook/flipper@ab121f9d9 (Flipper release tag v0.26.0); see
facebook/react-native#27565 (comment).
That issue was non-optimally resolved in
facebook/react-native@b4d1fcfb2, which we'll take for now in this
commit (see below); a more principled solution is open as
facebook/flipper#1171.

The `project.pbxproj` changes here aren't fully explained, but
here's what I think happened, and what I did about it:

- An empty Swift file was added, to rouse Xcode into being prepared
  to handle Swift files, which is needed for the "YogaKit" dep; see
  facebook/react-native#27426 (comment).
  This step is included in the upgrade guide [1]. The empty Swift
  file was later backed out in facebook/react-native@e3218a0d9 (see
  below), which is why it doesn't appear here.

- The addition of the empty Swift file activated a wizard that
  walked the user through setting up an Objective-C "bridging
  header" file and set some plausibly desired settings in the
  `project.pbxproj` file. RN deleted the bridging header file, but
  the settings remained.

  We had actually already set up a bridging header file a long time
  ago, in d38129b, and this setup wizard didn't activate when I
  created the empty Swift file. I tried to *get* it to activate (to
  better reproduce the settings changes) by deleting our bridging
  header file and re-adding the Swift file, but it didn't work.
  However, the settings that got added in d38129b seem to match
  these settings pretty closely; one notable difference is that a
  SWIFT_VERSION build setting is 4.2 instead of 5.0.

  In any case, the upgrade guide on the Xcode changes [1] only says
  the bridging-header wizard will "most likely" appear, and if it
  doesn't, just keep going.

- They added "English" under `knownRegions` in the project info. We
  have this already, and, in fact, it gets *removed* if I follow an
  automatic deprecation fix that Xcode suggests in the buildtime
  warnings (an instance of zulip#4112). So, ignore this.

----- facebook/react-native@e3218a0d9 -----

This Facebook commit looks fairly coherent, but it's a partial
reversion of facebook/react-native@898b1db6d (just above) with a
tweak on top of it. So, might as well put it into the same commit.

The `project.pbxproj` changes are as follows, as best as I can tell:

- The empty Swift file described just above is removed. This is not
  reflected in the upgrade guide on the Xcode changes [1], which
  still says you need to create the file.

- A `LIBRARY_SEARCH_PATHS` setting is added. This *is* reflected in
  the upgrade guide on the Xcode changes [1]; following those
  instructions does indeed generate a portion of the diff.

- An `LD_RUNPATH_SEARCH_PATHS` setting is added; this is *not*
  reflected in the upgrade guide. It's reportedly crucial; see
  facebook/react-native#27922 (comment).
  I copied the code.

- A `DEAD_CODE_STRIPPING` setting is removed; this is *not*
  reflected in the upgrade guide. Reportedly (and in my experience,
  as best as I can remember and conclude), it's crucial; see
  facebook/react-native#27922 (comment).
  I copied the code.

----- facebook/react-native@8f954b365 -----

Just simple prose adjustments to a comment, but it's substantially
reworked in facebook/react-native@b4d1fcfb2 (which follows).

----- facebook/react-native@b4d1fcfb2 -----

This looks like a partial reversion of
facebook/react-native@898b1db6d (above), with a tweak; the
`FB_SONARKIT_ENABLED` conditional is removed from `AppDelegate.m`,
but that identifier now appears in the `project.pbxproj`.

`ENABLE_BITCODE = NO;` is added to the `project.pbxproj` to fix an
issue introduced in facebook/react-native@898b1db6d (see above); see
facebook/react-native#27565 (comment).
The issue would be better resolved with something like
facebook/flipper#1171. In particular, I'm inclined to agree with
that PR's author when he says "it strikes me that not having access
to current (and possible future) updates of a cryptography library
is not a good long-term plan".

This commit also continues the advancement of the FlipperKit pod's
version, this time to v0.30.2, and adjusts some comments.

----- facebook/react-native@f6a8452e7 -----

After an upgrade to FlipperKit v0.32.0 in
facebook/react-native@ada73a354, quickly reverted in
facebook/react-native@4bb17944f, this commit brings us to v0.33.1.

----- facebook/react-native@8858d879e -----

This is the last commit that affects Flipper on iOS before the v0.62
release was made. It doesn't continue the advancement of the
FlipperKit version (and a good thing, too, maybe!). It lists several
transitive dependencies of FlipperKit pods to the Podfile and gives
them each a `:configuration => 'Debug'` setting, and that's it.

It appears to be accounting for a CocoaPods quirk that means that,
before this change is made, all those dependencies would get
included in release builds, increasing the binary size. Might as
well avoid having any commits with that increased size; so, conclude
this giant commit with these changes.

All that's left is to actually enable Flipper, which we'll do in the
main upgrade commit.

[1] react-native-community/upgrade-support#13
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Sep 2, 2020
We'll actually enable it in the main RN v0.61 -> v0.62 upgrade
commit, if only because running Flipper pre-RN v0.62 isn't
advertised as something you can do. Enabling it means calling the
initialize function, like in facebook/react-native@05f5cb534, but in
the form it took in facebook/react-native@b4d1fcfb2 (and the
conditional that appears there).

This encompasses several RN commits, as follows, some of which broke
builds (which poses a problem even without Flipper enabled). We lump
these in with their fixes, so that we don't knowingly introduce
errors at any commit in our project. Some of these problems can be
traced to upgrading the FlipperKit pod and not handling breaking
changes at the same time. Perhaps it wouldn't have been easy for RN
to catch these before merging.

Generally, though, the RN commits aren't very clear or coherent (at
least by our standards), so I feel safer flattening all of these
even if something could plausibly stand alone in our project.

----- facebook/react-native@70274f4e9 -----

This commit is unstable. One reason is that `pod install` fails
until `:modular_headers` is added to the `Yoga` pod declaration in
facebook/react-native@898b1db6d. The failure message:

```
[!] The following Swift pods cannot yet be integrated as static
libraries:

The Swift pod `YogaKit` depends upon `Yoga`, which does not define
modules. To opt into those targets generating module maps (which is
necessary to import them from Swift when building as static
libraries), you may set `use_modular_headers!` globally in your
Podfile, or specify `:modular_headers => true` for particular
dependencies.
```

----- facebook/react-native@e8541e03f -----

The claim is that this commit adds running React Dev Tools to the
template. I had thought this was something totally separate from
Flipper (and so this would naturally stand in its own commit), but
no; the added code comes from the `FlipperKitReactPlugin` subspec of
the `FlipperKit` pod, and it'll be invoked in the Flipper
initialization function.

This commit also advances the FlipperKit pod version to v0.23.6; I'm
not aware of any new issues that were specifically introduced in
this version.

Still, seems safest to apply these changes in this giant commit.

----- facebook/react-native@898b1db6d -----

A small handful of changes in `AppDelegate.m`, the Podfile, and
`project.pbxproj` are made here.

The motivation for the `AppDelegate.m` changes isn't clear to me,
unless it's "The integration of Flipper on iOS was crashing the
template"--not super descriptive or helpful in finding the origin of
the issue.

In the Podfile, the addition of `:modular_headers => true` to the
`Yoga` pod declaration fixes a `pod install` failure introduced in
facebook/react-native@70274f4e9, described above.

Also in the Podfile, the change of FlipperKit's version to v0.30.0
introduces a build failure associated with
facebook/flipper@ab121f9d9 (Flipper release tag v0.26.0); see
facebook/react-native#27565 (comment).
That issue was non-optimally resolved in
facebook/react-native@b4d1fcfb2, which we'll take for now in this
commit (see below); a more principled solution is open as
facebook/flipper#1171.

The `project.pbxproj` changes here aren't fully explained, but
here's what I think happened, and what I did about it:

- An empty Swift file was added, to rouse Xcode into being prepared
  to handle Swift files, which is needed for the "YogaKit" dep; see
  facebook/react-native#27426 (comment).
  This step is included in the upgrade guide [1]. The empty Swift
  file was later backed out in facebook/react-native@e3218a0d9 (see
  below), which is why it doesn't appear here.

- The addition of the empty Swift file activated a wizard that
  walked the user through setting up an Objective-C "bridging
  header" file and set some plausibly desired settings in the
  `project.pbxproj` file. RN deleted the bridging header file, but
  the settings remained.

  We had actually already set up a bridging header file a long time
  ago, in d38129b, and this setup wizard didn't activate when I
  created the empty Swift file. I tried to *get* it to activate (to
  better reproduce the settings changes) by deleting our bridging
  header file and re-adding the Swift file, but it didn't work.
  However, the settings that got added in d38129b seem to match
  these settings pretty closely; one notable difference is that a
  SWIFT_VERSION build setting is 4.2 instead of 5.0.

  In any case, the upgrade guide on the Xcode changes [1] only says
  the bridging-header wizard will "most likely" appear, and if it
  doesn't, just keep going.

- They added "English" under `knownRegions` in the project info. We
  have this already, and, in fact, it gets *removed* if I follow an
  automatic deprecation fix that Xcode suggests in the buildtime
  warnings (an instance of zulip#4112). So, ignore this.

----- facebook/react-native@e3218a0d9 -----

This Facebook commit looks fairly coherent, but it's a partial
reversion of facebook/react-native@898b1db6d (just above) with a
tweak on top of it. So, might as well put it into the same commit.

The `project.pbxproj` changes are as follows, as best as I can tell:

- The empty Swift file described just above is removed. This is not
  reflected in the upgrade guide on the Xcode changes [1], which
  still says you need to create the file.

- A `LIBRARY_SEARCH_PATHS` setting is added. This *is* reflected in
  the upgrade guide on the Xcode changes [1]; following those
  instructions does indeed generate a portion of the diff.

- An `LD_RUNPATH_SEARCH_PATHS` setting is added; this is *not*
  reflected in the upgrade guide. It's reportedly crucial; see
  facebook/react-native#27922 (comment).
  I copied the code.

- A `DEAD_CODE_STRIPPING` setting is removed; this is *not*
  reflected in the upgrade guide. Reportedly (and in my experience,
  as best as I can remember and conclude), it's crucial; see
  facebook/react-native#27922 (comment).
  I copied the code.

----- facebook/react-native@8f954b365 -----

Just simple prose adjustments to a comment, but it's substantially
reworked in facebook/react-native@b4d1fcfb2 (which follows).

----- facebook/react-native@b4d1fcfb2 -----

This looks like a partial reversion of
facebook/react-native@898b1db6d (above), with a tweak; the
`FB_SONARKIT_ENABLED` conditional is removed from `AppDelegate.m`,
but that identifier now appears in the `project.pbxproj`.

`ENABLE_BITCODE = NO;` is added to the `project.pbxproj` to fix an
issue introduced in facebook/react-native@898b1db6d (see above); see
facebook/react-native#27565 (comment).
The issue would be better resolved with something like
facebook/flipper#1171. In particular, I'm inclined to agree with
that PR's author when he says "it strikes me that not having access
to current (and possible future) updates of a cryptography library
is not a good long-term plan".

This commit also continues the advancement of the FlipperKit pod's
version, this time to v0.30.2, and adjusts some comments.

----- facebook/react-native@f6a8452e7 -----

After an upgrade to FlipperKit v0.32.0 in
facebook/react-native@ada73a354, quickly reverted in
facebook/react-native@4bb17944f, this commit brings us to v0.33.1.

----- facebook/react-native@8858d879e -----

This is the last commit that affects Flipper on iOS before the v0.62
release was made. It doesn't continue the advancement of the
FlipperKit version (and a good thing, too, maybe!). It lists several
transitive dependencies of FlipperKit pods to the Podfile and gives
them each a `:configuration => 'Debug'` setting, and that's it.

It appears to be accounting for a CocoaPods quirk that means that,
before this change is made, all those dependencies would get
included in release builds, increasing the binary size. Might as
well avoid having any commits with that increased size; so, conclude
this giant commit with these changes.

All that's left is to actually enable Flipper, which we'll do in the
main upgrade commit.

[1] react-native-community/upgrade-support#13
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Sep 2, 2020
We'll actually enable it in the main RN v0.61 -> v0.62 upgrade
commit, if only because running Flipper pre-RN v0.62 isn't
advertised as something you can do. Enabling it means calling the
initialize function, like in facebook/react-native@05f5cb534, but in
the form it took in facebook/react-native@b4d1fcfb2 (and the
conditional that appears there).

This encompasses several RN commits, as follows, some of which broke
builds (which poses a problem even without Flipper enabled). We lump
these in with their fixes, so that we don't knowingly introduce
errors at any commit in our project. Some of these problems can be
traced to upgrading the FlipperKit pod and not handling breaking
changes at the same time. Perhaps it wouldn't have been easy for RN
to catch these before merging.

Generally, though, the RN commits aren't very clear or coherent (at
least by our standards), so I feel safer flattening all of these
even if something could plausibly stand alone in our project.

----- facebook/react-native@70274f4e9 -----

This commit is unstable. One reason is that `pod install` fails
until `:modular_headers` is added to the `Yoga` pod declaration in
facebook/react-native@898b1db6d. The failure message:

```
[!] The following Swift pods cannot yet be integrated as static
libraries:

The Swift pod `YogaKit` depends upon `Yoga`, which does not define
modules. To opt into those targets generating module maps (which is
necessary to import them from Swift when building as static
libraries), you may set `use_modular_headers!` globally in your
Podfile, or specify `:modular_headers => true` for particular
dependencies.
```

----- facebook/react-native@e8541e03f -----

The claim is that this commit adds running React Dev Tools to the
template. I had thought this was something totally separate from
Flipper (and so this would naturally stand in its own commit), but
no; the added code comes from the `FlipperKitReactPlugin` subspec of
the `FlipperKit` pod, and it'll be invoked in the Flipper
initialization function.

This commit also advances the FlipperKit pod version to v0.23.6; I'm
not aware of any new issues that were specifically introduced in
this version.

Still, seems safest to apply these changes in this giant commit.

----- facebook/react-native@898b1db6d -----

A small handful of changes in `AppDelegate.m`, the Podfile, and
`project.pbxproj` are made here.

The motivation for the `AppDelegate.m` changes isn't clear to me,
unless it's "The integration of Flipper on iOS was crashing the
template"--not super descriptive or helpful in finding the origin of
the issue.

In the Podfile, the addition of `:modular_headers => true` to the
`Yoga` pod declaration fixes a `pod install` failure introduced in
facebook/react-native@70274f4e9, described above.

Also in the Podfile, the change of FlipperKit's version to v0.30.0
introduces a build failure associated with
facebook/flipper@ab121f9d9 (Flipper release tag v0.26.0); see
facebook/react-native#27565 (comment).
That issue was non-optimally resolved in
facebook/react-native@b4d1fcfb2, which we'll take for now in this
commit (see below); a more principled solution is open as
facebook/flipper#1171.

The `project.pbxproj` changes here aren't fully explained, but
here's what I think happened, and what I did about it:

- An empty Swift file was added, to rouse Xcode into being prepared
  to handle Swift files, which is needed for the "YogaKit" dep; see
  facebook/react-native#27426 (comment).
  This step is included in the upgrade guide [1]. The empty Swift
  file was later backed out in facebook/react-native@e3218a0d9 (see
  below), which is why it doesn't appear here.

- The addition of the empty Swift file activated a wizard that
  walked the user through setting up an Objective-C "bridging
  header" file and set some plausibly desired settings in the
  `project.pbxproj` file. RN deleted the bridging header file, but
  the settings remained.

  We had actually already set up a bridging header file a long time
  ago, in d38129b, and this setup wizard didn't activate when I
  created the empty Swift file. I tried to *get* it to activate (to
  better reproduce the settings changes) by deleting our bridging
  header file and re-adding the Swift file, but it didn't work.
  However, the settings that got added in d38129b seem to match
  these settings pretty closely; one notable difference is that a
  SWIFT_VERSION build setting is 4.2 instead of 5.0.

  In any case, the upgrade guide on the Xcode changes [1] only says
  the bridging-header wizard will "most likely" appear, and if it
  doesn't, just keep going.

- They added "English" under `knownRegions` in the project info. We
  have this already, and, in fact, it gets *removed* if I follow an
  automatic deprecation fix that Xcode suggests in the buildtime
  warnings (an instance of zulip#4112). So, ignore this.

----- facebook/react-native@e3218a0d9 -----

This Facebook commit looks fairly coherent, but it's a partial
reversion of facebook/react-native@898b1db6d (just above) with a
tweak on top of it. So, might as well put it into the same commit.

The `project.pbxproj` changes are as follows, as best as I can tell:

- The empty Swift file described just above is removed. This is not
  reflected in the upgrade guide on the Xcode changes [1], which
  still says you need to create the file.

- A `LIBRARY_SEARCH_PATHS` setting is added. This *is* reflected in
  the upgrade guide on the Xcode changes [1]; following those
  instructions does indeed generate a portion of the diff.

- An `LD_RUNPATH_SEARCH_PATHS` setting is added; this is *not*
  reflected in the upgrade guide. It's reportedly crucial; see
  facebook/react-native#27922 (comment).
  I copied the code.

- A `DEAD_CODE_STRIPPING` setting is removed; this is *not*
  reflected in the upgrade guide. Reportedly (and in my experience,
  as best as I can remember and conclude), it's crucial; see
  facebook/react-native#27922 (comment).
  I copied the code.

----- facebook/react-native@8f954b365 -----

Just simple prose adjustments to a comment, but it's substantially
reworked in facebook/react-native@b4d1fcfb2 (which follows).

----- facebook/react-native@b4d1fcfb2 -----

This looks like a partial reversion of
facebook/react-native@898b1db6d (above), with a tweak; the
`FB_SONARKIT_ENABLED` conditional is removed from `AppDelegate.m`,
but that identifier now appears in the `project.pbxproj`.

`ENABLE_BITCODE = NO;` is added to the `project.pbxproj` to fix an
issue introduced in facebook/react-native@898b1db6d (see above); see
facebook/react-native#27565 (comment).
The issue would be better resolved with something like
facebook/flipper#1171. In particular, I'm inclined to agree with
that PR's author when he says "it strikes me that not having access
to current (and possible future) updates of a cryptography library
is not a good long-term plan".

This commit also continues the advancement of the FlipperKit pod's
version, this time to v0.30.2, and adjusts some comments.

----- facebook/react-native@f6a8452e7 -----

After an upgrade to FlipperKit v0.32.0 in
facebook/react-native@ada73a354, quickly reverted in
facebook/react-native@4bb17944f, this commit brings us to v0.33.1.

----- facebook/react-native@8858d879e -----

This is the last commit that affects Flipper on iOS before the v0.62
release was made. It doesn't continue the advancement of the
FlipperKit version (and a good thing, too, maybe!). It lists several
transitive dependencies of FlipperKit pods to the Podfile and gives
them each a `:configuration => 'Debug'` setting, and that's it.

It appears to be accounting for a CocoaPods quirk that means that,
before this change is made, all those dependencies would get
included in release builds, increasing the binary size. Might as
well avoid having any commits with that increased size; so, conclude
this giant commit with these changes.

All that's left is to actually enable Flipper, which we'll do in the
main upgrade commit.

[1] react-native-community/upgrade-support#13
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Sep 4, 2020
We'll actually enable it after the main RN v0.61 -> v0.62 upgrade
commit, if only because running Flipper pre-RN v0.62 isn't
advertised as something you can do. Enabling it means calling the
initialize function, like in facebook/react-native@05f5cb534, but in
the form it took in facebook/react-native@b4d1fcfb2 (and in the
conditional that appears there).

This encompasses several RN commits, as follows, some of which broke
builds (which poses a problem even without Flipper enabled). We lump
these in with their fixes, so that we don't knowingly introduce
errors at any commit in our project. Some of these problems can be
traced to upgrading the FlipperKit pod and not handling breaking
changes at the same time. Perhaps it wouldn't have been easy for RN
to catch these before merging.

Generally, though, the RN commits aren't very clear or coherent (at
least by our standards), so I feel safer flattening all of these
even if something could plausibly stand alone in our project.

----- facebook/react-native@70274f4e9 -----

This commit is unstable. One reason is that `pod install` fails
until `:modular_headers` is added to the `Yoga` pod declaration in
facebook/react-native@898b1db6d. The failure message:

```
[!] The following Swift pods cannot yet be integrated as static
libraries:

The Swift pod `YogaKit` depends upon `Yoga`, which does not define
modules. To opt into those targets generating module maps (which is
necessary to import them from Swift when building as static
libraries), you may set `use_modular_headers!` globally in your
Podfile, or specify `:modular_headers => true` for particular
dependencies.
```

----- facebook/react-native@e8541e03f -----

The claim is that this commit adds running React Dev Tools to the
template. I had thought this was something totally separate from
Flipper (and so this would naturally stand in its own commit), but
no; the added code comes from the `FlipperKitReactPlugin` subspec of
the `FlipperKit` pod, and it'll be invoked in the Flipper
initialization function.

This commit also advances the FlipperKit pod version to v0.23.6; I'm
not aware of any new issues that were specifically introduced in
this version.

Still, seems safest to apply these changes in this giant commit.

----- facebook/react-native@898b1db6d -----

A small handful of changes in `AppDelegate.m`, the Podfile, and
`project.pbxproj` are made here.

The motivation for the `AppDelegate.m` changes isn't clear to me,
unless it's "The integration of Flipper on iOS was crashing the
template"--not super descriptive or helpful in finding the origin of
the issue.

In the Podfile, the addition of `:modular_headers => true` to the
`Yoga` pod declaration fixes a `pod install` failure introduced in
facebook/react-native@70274f4e9, described above.

Also in the Podfile, the change of FlipperKit's version to v0.30.0
introduces a build failure associated with
facebook/flipper@ab121f9d9 (Flipper release tag v0.26.0); see
facebook/react-native#27565 (comment).
That issue was non-optimally resolved in
facebook/react-native@b4d1fcfb2, which we take for now in this
commit (see below); a more principled solution is open as
facebook/flipper#1171.

The `project.pbxproj` changes here aren't fully explained, but
here's what I think happened, and what I did about it:

- An empty Swift file was added, to rouse Xcode into being prepared
  to handle Swift files, which is needed for the "YogaKit" dep; see
  facebook/react-native#27426 (comment).
  This step is included in the upgrade guide [1]. The empty Swift
  file was later backed out in facebook/react-native@e3218a0d9 (see
  below), which is why it doesn't appear here.

- The addition of the empty Swift file activated a wizard that
  walked the user through setting up an Objective-C "bridging
  header" file and set some plausibly desired settings in the
  `project.pbxproj` file. RN deleted the bridging header file, but
  the settings remained.

  We had actually already set up a bridging header file a long time
  ago, in d38129b, and this setup wizard didn't activate when I
  created the empty Swift file. I tried to *get* it to activate (to
  better reproduce the settings changes) by deleting our bridging
  header file and re-adding the Swift file, but the wizard didn't
  activate. However, the settings that got added in d38129b seem
  to match these settings pretty closely; one notable difference is
  that a SWIFT_VERSION build setting is 4.2 instead of 5.0.

  In any case, the upgrade guide on the Xcode changes [1] only says
  the bridging-header wizard will "most likely" appear, and if it
  doesn't, just keep going.

- They added "English" under `knownRegions` in the project info. It
  appears Xcode doesn't like this change; it gets reverted later as
  part of facebook/react-native@ebb629d05 (which we take elsewhere
  in this series, before the upgrade, as an instance of zulip#4112), when
  they follow an Xcode automatic deprecation fix. So, ignore this.

----- facebook/react-native@e3218a0d9 -----

This Facebook commit looks fairly coherent, but it's a partial
reversion of facebook/react-native@898b1db6d (just above) with a
tweak on top of it. So, might as well put it into the same commit.

The `project.pbxproj` changes are as follows, as best as I can tell:

- The empty Swift file described just above is removed. The commit
  message makes it sound like it's no longer necessary for projects
  to make the Swift file in the first place; on the other hand, the
  upgrade guide on the Xcode changes [1] indicate that it should be
  created and then removed.

- A `LIBRARY_SEARCH_PATHS` setting is added. This *is* reflected in
  the upgrade guide on the Xcode changes [1]; following those
  instructions does indeed generate a portion of the diff.

- An `LD_RUNPATH_SEARCH_PATHS` setting is added; this is *not*
  reflected in the upgrade guide. It's reportedly crucial; see
  facebook/react-native#27922 (comment).
  I copied the code.

- A `DEAD_CODE_STRIPPING` setting is removed; this is *not*
  reflected in the upgrade guide. Reportedly (and in my experience,
  as best as I can remember and conclude), it's crucial; see
  facebook/react-native#27922 (comment).
  I copied the code.

----- facebook/react-native@8f954b365 -----

Just simple prose adjustments to a comment, but it's substantially
reworked in facebook/react-native@b4d1fcfb2 (which follows).

----- facebook/react-native@b4d1fcfb2 -----

This looks like a partial reversion of
facebook/react-native@898b1db6d (above), with a tweak; the
`FB_SONARKIT_ENABLED` conditional is removed from `AppDelegate.m`,
but that identifier now appears in the `project.pbxproj`.

`ENABLE_BITCODE = NO;` is added to the `project.pbxproj` to fix an
issue introduced in facebook/react-native@898b1db6d (see above); see
facebook/react-native#27565 (comment).
The issue would be better resolved with something like
facebook/flipper#1171. In particular, I'm inclined to agree with
that PR's author, who says "it strikes me that not having access to
current (and possible future) updates of a cryptography library is
not a good long-term plan".

This commit also continues the advancement of the FlipperKit pod's
version, this time to v0.30.2, and adjusts some comments.

----- facebook/react-native@f6a8452e7 -----

After an upgrade to FlipperKit v0.32.0 in
facebook/react-native@ada73a354, quickly reverted in
facebook/react-native@4bb17944f, this commit brings us to v0.33.1.

----- facebook/react-native@8858d879e -----

This is the last commit that affects Flipper on iOS before the v0.62
release was made. It doesn't continue the advancement of the
FlipperKit version (and a good thing, too, maybe!). It lists several
transitive dependencies of FlipperKit pods to the Podfile and gives
them each a `:configuration => 'Debug'` setting, and that's it.

It appears to be accounting for a CocoaPods quirk that means that,
before this change is made, all those dependencies would get
included in release builds, increasing the binary size. Might as
well avoid having any commits with that increased size; so, conclude
this giant commit with these changes.

All that's left is to actually enable Flipper, which we'll do after
the main upgrade commit; see
zulip#4244 (comment).

[1] react-native-community/upgrade-support#13
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Sep 9, 2020
We'll actually enable it after the main RN v0.61 -> v0.62 upgrade
commit, if only because running Flipper pre-RN v0.62 isn't
advertised as something you can do. Enabling it means calling the
initialize function, like in facebook/react-native@05f5cb534, but in
the form it took in facebook/react-native@b4d1fcfb2 (and in the
conditional that appears there).

This encompasses several RN commits, as follows, some of which broke
builds (which poses a problem even without Flipper enabled). We lump
these in with their fixes, so that we don't knowingly introduce
errors at any commit in our project. Some of these problems can be
traced to upgrading the FlipperKit pod and not handling breaking
changes at the same time. Perhaps it wouldn't have been easy for RN
to catch these before merging.

Generally, though, the RN commits aren't very clear or coherent (at
least by our standards), so I feel safer flattening all of these
even if something could plausibly stand alone in our project.

----- facebook/react-native@70274f4e9 -----

This commit is unstable. One reason is that `pod install` fails
until `:modular_headers` is added to the `Yoga` pod declaration in
facebook/react-native@898b1db6d. The failure message:

```
[!] The following Swift pods cannot yet be integrated as static
libraries:

The Swift pod `YogaKit` depends upon `Yoga`, which does not define
modules. To opt into those targets generating module maps (which is
necessary to import them from Swift when building as static
libraries), you may set `use_modular_headers!` globally in your
Podfile, or specify `:modular_headers => true` for particular
dependencies.
```

----- facebook/react-native@e8541e03f -----

The claim is that this commit adds running React Dev Tools to the
template. I had thought this was something totally separate from
Flipper (and so this would naturally stand in its own commit), but
no; the added code comes from the `FlipperKitReactPlugin` subspec of
the `FlipperKit` pod, and it'll be invoked in the Flipper
initialization function.

This commit also advances the FlipperKit pod version to v0.23.6; I'm
not aware of any new issues that were specifically introduced in
this version.

Still, seems safest to apply these changes in this giant commit.

----- facebook/react-native@898b1db6d -----

A small handful of changes in `AppDelegate.m`, the Podfile, and
`project.pbxproj` are made here.

The motivation for the `AppDelegate.m` changes isn't clear to me,
unless it's "The integration of Flipper on iOS was crashing the
template"--not super descriptive or helpful in finding the origin of
the issue.

In the Podfile, the addition of `:modular_headers => true` to the
`Yoga` pod declaration fixes a `pod install` failure introduced in
facebook/react-native@70274f4e9, described above.

Also in the Podfile, the change of FlipperKit's version to v0.30.0
introduces a build failure associated with
facebook/flipper@ab121f9d9 (Flipper release tag v0.26.0); see
facebook/react-native#27565 (comment).
That issue was non-optimally resolved in
facebook/react-native@b4d1fcfb2, which we take for now in this
commit (see below); a more principled solution is open as
facebook/flipper#1171.

The `project.pbxproj` changes here aren't fully explained, but
here's what I think happened, and what I did about it:

- An empty Swift file was added, to rouse Xcode into being prepared
  to handle Swift files, which is needed for the "YogaKit" dep; see
  facebook/react-native#27426 (comment).
  This step is included in the upgrade guide [1]. The empty Swift
  file was later backed out in facebook/react-native@e3218a0d9 (see
  below), which is why it doesn't appear here.

- The addition of the empty Swift file activated a wizard that
  walked the user through setting up an Objective-C "bridging
  header" file and set some plausibly desired settings in the
  `project.pbxproj` file. RN deleted the bridging header file, but
  the settings remained.

  We had actually already set up a bridging header file a long time
  ago, in d38129b, and this setup wizard didn't activate when I
  created the empty Swift file. I tried to *get* it to activate (to
  better reproduce the settings changes) by deleting our bridging
  header file and re-adding the Swift file, but the wizard didn't
  activate. However, the settings that got added in d38129b seem
  to match these settings pretty closely; one notable difference is
  that a SWIFT_VERSION build setting is 4.2 instead of 5.0.

  In any case, the upgrade guide on the Xcode changes [1] only says
  the bridging-header wizard will "most likely" appear, and if it
  doesn't, just keep going.

- They added "English" under `knownRegions` in the project info. It
  appears Xcode doesn't like this change; it gets reverted later as
  part of facebook/react-native@ebb629d05 (which we take elsewhere
  in this series, before the upgrade, as an instance of zulip#4112), when
  they follow an Xcode automatic deprecation fix. So, ignore this.

----- facebook/react-native@e3218a0d9 -----

This Facebook commit looks fairly coherent, but it's a partial
reversion of facebook/react-native@898b1db6d (just above) with a
tweak on top of it. So, might as well put it into the same commit.

The `project.pbxproj` changes are as follows, as best as I can tell:

- The empty Swift file described just above is removed. The commit
  message makes it sound like it's no longer necessary for projects
  to make the Swift file in the first place; on the other hand, the
  upgrade guide on the Xcode changes [1] indicate that it should be
  created and then removed.

- A `LIBRARY_SEARCH_PATHS` setting is added. This *is* reflected in
  the upgrade guide on the Xcode changes [1]; following those
  instructions does indeed generate a portion of the diff.

- An `LD_RUNPATH_SEARCH_PATHS` setting is added; this is *not*
  reflected in the upgrade guide. It's reportedly crucial; see
  facebook/react-native#27922 (comment).
  I copied the code.

- A `DEAD_CODE_STRIPPING` setting is removed; this is *not*
  reflected in the upgrade guide. Reportedly (and in my experience,
  as best as I can remember and conclude), it's crucial; see
  facebook/react-native#27922 (comment).
  I copied the code.

----- facebook/react-native@8f954b365 -----

Just simple prose adjustments to a comment, but it's substantially
reworked in facebook/react-native@b4d1fcfb2 (which follows).

----- facebook/react-native@b4d1fcfb2 -----

This looks like a partial reversion of
facebook/react-native@898b1db6d (above), with a tweak; the
`FB_SONARKIT_ENABLED` conditional is removed from `AppDelegate.m`,
but that identifier now appears in the `project.pbxproj`.

`ENABLE_BITCODE = NO;` is added to the `project.pbxproj` to fix an
issue introduced in facebook/react-native@898b1db6d (see above); see
facebook/react-native#27565 (comment).
The issue would be better resolved with something like
facebook/flipper#1171. In particular, I'm inclined to agree with
that PR's author, who says "it strikes me that not having access to
current (and possible future) updates of a cryptography library is
not a good long-term plan".

This commit also continues the advancement of the FlipperKit pod's
version, this time to v0.30.2, and adjusts some comments.

----- facebook/react-native@f6a8452e7 -----

After an upgrade to FlipperKit v0.32.0 in
facebook/react-native@ada73a354, quickly reverted in
facebook/react-native@4bb17944f, this commit brings us to v0.33.1.

----- facebook/react-native@8858d879e -----

This is the last commit that affects Flipper on iOS before the v0.62
release was made. It doesn't continue the advancement of the
FlipperKit version (and a good thing, too, maybe!). It lists several
transitive dependencies of FlipperKit pods to the Podfile and gives
them each a `:configuration => 'Debug'` setting, and that's it.

It appears to be accounting for a CocoaPods quirk that means that,
before this change is made, all those dependencies would get
included in release builds, increasing the binary size. Might as
well avoid having any commits with that increased size; so, conclude
this giant commit with these changes.

All that's left is to actually enable Flipper, which we'll do after
the main upgrade commit; see
zulip#4244 (comment).

[1] react-native-community/upgrade-support#13
gnprice pushed a commit to chrisbobbe/zulip-mobile that referenced this pull request Sep 11, 2020
We'll actually enable it after the main RN v0.61 -> v0.62 upgrade
commit, if only because running Flipper pre-RN v0.62 isn't
advertised as something you can do. Enabling it means calling the
initialize function, like in facebook/react-native@05f5cb534, but in
the form it took in facebook/react-native@b4d1fcfb2 (and in the
conditional that appears there).

This encompasses several RN commits, as follows, some of which broke
builds (which poses a problem even without Flipper enabled). We lump
these in with their fixes, so that we don't knowingly introduce
errors at any commit in our project. Some of these problems can be
traced to upgrading the FlipperKit pod and not handling breaking
changes at the same time. Perhaps it wouldn't have been easy for RN
to catch these before merging.

Generally, though, the RN commits aren't very clear or coherent (at
least by our standards), so I feel safer flattening all of these
even if something could plausibly stand alone in our project.

----- facebook/react-native@70274f4e9 -----

This commit is unstable. One reason is that `pod install` fails
until `:modular_headers` is added to the `Yoga` pod declaration in
facebook/react-native@898b1db6d. The failure message:

```
[!] The following Swift pods cannot yet be integrated as static
libraries:

The Swift pod `YogaKit` depends upon `Yoga`, which does not define
modules. To opt into those targets generating module maps (which is
necessary to import them from Swift when building as static
libraries), you may set `use_modular_headers!` globally in your
Podfile, or specify `:modular_headers => true` for particular
dependencies.
```

----- facebook/react-native@e8541e03f -----

The claim is that this commit adds running React Dev Tools to the
template. I had thought this was something totally separate from
Flipper (and so this would naturally stand in its own commit), but
no; the added code comes from the `FlipperKitReactPlugin` subspec of
the `FlipperKit` pod, and it'll be invoked in the Flipper
initialization function.

This commit also advances the FlipperKit pod version to v0.23.6; I'm
not aware of any new issues that were specifically introduced in
this version.

Still, seems safest to apply these changes in this giant commit.

----- facebook/react-native@898b1db6d -----

A small handful of changes in `AppDelegate.m`, the Podfile, and
`project.pbxproj` are made here.

The motivation for the `AppDelegate.m` changes isn't clear to me,
unless it's "The integration of Flipper on iOS was crashing the
template"--not super descriptive or helpful in finding the origin of
the issue.

In the Podfile, the addition of `:modular_headers => true` to the
`Yoga` pod declaration fixes a `pod install` failure introduced in
facebook/react-native@70274f4e9, described above.

Also in the Podfile, the change of FlipperKit's version to v0.30.0
introduces a build failure associated with
facebook/flipper@ab121f9d9 (Flipper release tag v0.26.0); see
facebook/react-native#27565 (comment).
That issue was non-optimally resolved in
facebook/react-native@b4d1fcfb2, which we take for now in this
commit (see below); a more principled solution is open as
facebook/flipper#1171.

The `project.pbxproj` changes here aren't fully explained, but
here's what I think happened, and what I did about it:

- An empty Swift file was added, to rouse Xcode into being prepared
  to handle Swift files, which is needed for the "YogaKit" dep; see
  facebook/react-native#27426 (comment).
  This step is included in the upgrade guide [1]. The empty Swift
  file was later backed out in facebook/react-native@e3218a0d9 (see
  below), which is why it doesn't appear here.

- The addition of the empty Swift file activated a wizard that
  walked the user through setting up an Objective-C "bridging
  header" file and set some plausibly desired settings in the
  `project.pbxproj` file. RN deleted the bridging header file, but
  the settings remained.

  We had actually already set up a bridging header file a long time
  ago, in d38129b, and this setup wizard didn't activate when I
  created the empty Swift file. I tried to *get* it to activate (to
  better reproduce the settings changes) by deleting our bridging
  header file and re-adding the Swift file, but the wizard didn't
  activate. However, the settings that got added in d38129b seem
  to match these settings pretty closely; one notable difference is
  that a SWIFT_VERSION build setting is 4.2 instead of 5.0.

  In any case, the upgrade guide on the Xcode changes [1] only says
  the bridging-header wizard will "most likely" appear, and if it
  doesn't, just keep going.

- They added "English" under `knownRegions` in the project info. It
  appears Xcode doesn't like this change; it gets reverted later as
  part of facebook/react-native@ebb629d05 (which we take elsewhere
  in this series, before the upgrade, as an instance of zulip#4112), when
  they follow an Xcode automatic deprecation fix. So, ignore this.

----- facebook/react-native@e3218a0d9 -----

This Facebook commit looks fairly coherent, but it's a partial
reversion of facebook/react-native@898b1db6d (just above) with a
tweak on top of it. So, might as well put it into the same commit.

The `project.pbxproj` changes are as follows, as best as I can tell:

- The empty Swift file described just above is removed. The commit
  message makes it sound like it's no longer necessary for projects
  to make the Swift file in the first place; on the other hand, the
  upgrade guide on the Xcode changes [1] indicate that it should be
  created and then removed.

- A `LIBRARY_SEARCH_PATHS` setting is added. This *is* reflected in
  the upgrade guide on the Xcode changes [1]; following those
  instructions does indeed generate a portion of the diff.

- An `LD_RUNPATH_SEARCH_PATHS` setting is added; this is *not*
  reflected in the upgrade guide. It's reportedly crucial; see
  facebook/react-native#27922 (comment).
  I copied the code.

- A `DEAD_CODE_STRIPPING` setting is removed; this is *not*
  reflected in the upgrade guide. Reportedly (and in my experience,
  as best as I can remember and conclude), it's crucial; see
  facebook/react-native#27922 (comment).
  I copied the code.

----- facebook/react-native@8f954b365 -----

Just simple prose adjustments to a comment, but it's substantially
reworked in facebook/react-native@b4d1fcfb2 (which follows).

----- facebook/react-native@b4d1fcfb2 -----

This looks like a partial reversion of
facebook/react-native@898b1db6d (above), with a tweak; the
`FB_SONARKIT_ENABLED` conditional is removed from `AppDelegate.m`,
but that identifier now appears in the `project.pbxproj`.

`ENABLE_BITCODE = NO;` is added to the `project.pbxproj` to fix an
issue introduced in facebook/react-native@898b1db6d (see above); see
facebook/react-native#27565 (comment).
The issue would be better resolved with something like
facebook/flipper#1171. In particular, I'm inclined to agree with
that PR's author, who says "it strikes me that not having access to
current (and possible future) updates of a cryptography library is
not a good long-term plan".

This commit also continues the advancement of the FlipperKit pod's
version, this time to v0.30.2, and adjusts some comments.

----- facebook/react-native@f6a8452e7 -----

After an upgrade to FlipperKit v0.32.0 in
facebook/react-native@ada73a354, quickly reverted in
facebook/react-native@4bb17944f, this commit brings us to v0.33.1.

----- facebook/react-native@8858d879e -----

This is the last commit that affects Flipper on iOS before the v0.62
release was made. It doesn't continue the advancement of the
FlipperKit version (and a good thing, too, maybe!). It lists several
transitive dependencies of FlipperKit pods to the Podfile and gives
them each a `:configuration => 'Debug'` setting, and that's it.

It appears to be accounting for a CocoaPods quirk that means that,
before this change is made, all those dependencies would get
included in release builds, increasing the binary size. Might as
well avoid having any commits with that increased size; so, conclude
this giant commit with these changes.

All that's left is to actually enable Flipper, which we'll do after
the main upgrade commit; see
zulip#4244 (comment).

[1] react-native-community/upgrade-support#13
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Sep 11, 2020
We'll actually enable it after the main RN v0.61 -> v0.62 upgrade
commit, if only because running Flipper pre-RN v0.62 isn't
advertised as something you can do. Enabling it means calling the
initialize function, like in facebook/react-native@05f5cb534, but in
the form it took in facebook/react-native@b4d1fcfb2 (and in the
conditional that appears there).

This encompasses several RN commits, as follows, some of which broke
builds (which poses a problem even without Flipper enabled). We lump
these in with their fixes, so that we don't knowingly introduce
errors at any commit in our project. Some of these problems can be
traced to upgrading the FlipperKit pod and not handling breaking
changes at the same time. Perhaps it wouldn't have been easy for RN
to catch these before merging.

Generally, though, the RN commits aren't very clear or coherent (at
least by our standards), so I feel safer flattening all of these
even if something could plausibly stand alone in our project.

----- facebook/react-native@70274f4e9 -----

This commit is unstable. One reason is that `pod install` fails
until `:modular_headers` is added to the `Yoga` pod declaration in
facebook/react-native@898b1db6d. The failure message:

```
[!] The following Swift pods cannot yet be integrated as static
libraries:

The Swift pod `YogaKit` depends upon `Yoga`, which does not define
modules. To opt into those targets generating module maps (which is
necessary to import them from Swift when building as static
libraries), you may set `use_modular_headers!` globally in your
Podfile, or specify `:modular_headers => true` for particular
dependencies.
```

----- facebook/react-native@e8541e03f -----

The claim is that this commit adds running React Dev Tools to the
template. I had thought this was something totally separate from
Flipper (and so this would naturally stand in its own commit), but
no; the added code comes from the `FlipperKitReactPlugin` subspec of
the `FlipperKit` pod, and it'll be invoked in the Flipper
initialization function.

This commit also advances the FlipperKit pod version to v0.23.6; I'm
not aware of any new issues that were specifically introduced in
this version.

Still, seems safest to apply these changes in this giant commit.

----- facebook/react-native@898b1db6d -----

A small handful of changes in `AppDelegate.m`, the Podfile, and
`project.pbxproj` are made here.

The motivation for the `AppDelegate.m` changes isn't clear to me,
unless it's "The integration of Flipper on iOS was crashing the
template"--not super descriptive or helpful in finding the origin of
the issue.

In the Podfile, the addition of `:modular_headers => true` to the
`Yoga` pod declaration fixes a `pod install` failure introduced in
facebook/react-native@70274f4e9, described above.

Also in the Podfile, the change of FlipperKit's version to v0.30.0
introduces a build failure associated with
facebook/flipper@ab121f9d9 (Flipper release tag v0.26.0); see
facebook/react-native#27565 (comment).
That issue was non-optimally resolved in
facebook/react-native@b4d1fcfb2, which we take for now in this
commit (see below); a more principled solution is open as
facebook/flipper#1171.

The `project.pbxproj` changes here aren't fully explained, but
here's what I think happened, and what I did about it:

- An empty Swift file was added, to rouse Xcode into being prepared
  to handle Swift files, which is needed for the "YogaKit" dep; see
  facebook/react-native#27426 (comment).
  This step is included in the upgrade guide [1]. The empty Swift
  file was later backed out in facebook/react-native@e3218a0d9 (see
  below), which is why it doesn't appear here.

- The addition of the empty Swift file activated a wizard that
  walked the user through setting up an Objective-C "bridging
  header" file and set some plausibly desired settings in the
  `project.pbxproj` file. RN deleted the bridging header file, but
  the settings remained.

  We had actually already set up a bridging header file a long time
  ago, in d38129b, and this setup wizard didn't activate when I
  created the empty Swift file. I tried to *get* it to activate (to
  better reproduce the settings changes) by deleting our bridging
  header file and re-adding the Swift file, but the wizard didn't
  activate. However, the settings that got added in d38129b seem
  to match these settings pretty closely; one notable difference is
  that a SWIFT_VERSION build setting is 4.2 instead of 5.0.

  In any case, the upgrade guide on the Xcode changes [1] only says
  the bridging-header wizard will "most likely" appear, and if it
  doesn't, just keep going.

- They added "English" under `knownRegions` in the project info. It
  appears Xcode doesn't like this change; it gets reverted later as
  part of facebook/react-native@ebb629d05 (which we take elsewhere
  in this series, before the upgrade, as an instance of zulip#4112), when
  they follow an Xcode automatic deprecation fix. So, ignore this.

----- facebook/react-native@e3218a0d9 -----

This Facebook commit looks fairly coherent, but it's a partial
reversion of facebook/react-native@898b1db6d (just above) with a
tweak on top of it. So, might as well put it into the same commit.

The `project.pbxproj` changes are as follows, as best as I can tell:

- The empty Swift file described just above is removed. The commit
  message makes it sound like it's no longer necessary for projects
  to make the Swift file in the first place; on the other hand, the
  upgrade guide on the Xcode changes [1] indicate that it should be
  created and then removed.

- A `LIBRARY_SEARCH_PATHS` setting is added. This *is* reflected in
  the upgrade guide on the Xcode changes [1]; following those
  instructions does indeed generate a portion of the diff.

- An `LD_RUNPATH_SEARCH_PATHS` setting is added; this is *not*
  reflected in the upgrade guide. It's reportedly crucial; see
  facebook/react-native#27922 (comment).
  I copied the code.

- A `DEAD_CODE_STRIPPING` setting is removed; this is *not*
  reflected in the upgrade guide. Reportedly (and in my experience,
  as best as I can remember and conclude), it's crucial; see
  facebook/react-native#27922 (comment).
  I copied the code.

----- facebook/react-native@8f954b365 -----

Just simple prose adjustments to a comment, but it's substantially
reworked in facebook/react-native@b4d1fcfb2 (which follows).

----- facebook/react-native@b4d1fcfb2 -----

This looks like a partial reversion of
facebook/react-native@898b1db6d (above), with a tweak; the
`FB_SONARKIT_ENABLED` conditional is removed from `AppDelegate.m`,
but that identifier now appears in the `project.pbxproj`.

`ENABLE_BITCODE = NO;` is added to the `project.pbxproj` to fix an
issue introduced in facebook/react-native@898b1db6d (see above); see
facebook/react-native#27565 (comment).
The issue would be better resolved with something like
facebook/flipper#1171. In particular, I'm inclined to agree with
that PR's author, who says "it strikes me that not having access to
current (and possible future) updates of a cryptography library is
not a good long-term plan".

This commit also continues the advancement of the FlipperKit pod's
version, this time to v0.30.2, and adjusts some comments.

----- facebook/react-native@f6a8452e7 -----

After an upgrade to FlipperKit v0.32.0 in
facebook/react-native@ada73a354, quickly reverted in
facebook/react-native@4bb17944f, this commit brings us to v0.33.1.

----- facebook/react-native@8858d879e -----

This is the last commit that affects Flipper on iOS before the v0.62
release was made. It doesn't continue the advancement of the
FlipperKit version (and a good thing, too, maybe!). It lists several
transitive dependencies of FlipperKit pods to the Podfile and gives
them each a `:configuration => 'Debug'` setting, and that's it.

It appears to be accounting for a CocoaPods quirk that means that,
before this change is made, all those dependencies would get
included in release builds, increasing the binary size. Might as
well avoid having any commits with that increased size; so, conclude
this giant commit with these changes.

All that's left is to actually enable Flipper, which we'll do after
the main upgrade commit; see
zulip#4244 (comment).

[1] react-native-community/upgrade-support#13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Contributor A React Native contributor. Merged This PR has been merged. Platform: iOS iOS applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants