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

Support "Sign In with Apple" auth [iOS] #3964

Closed
gnprice opened this issue Mar 10, 2020 · 11 comments · Fixed by #4034
Closed

Support "Sign In with Apple" auth [iOS] #3964

gnprice opened this issue Mar 10, 2020 · 11 comments · Fixed by #4034
Assignees
Labels
a-iOS a-onboarding Everything you would do when first joining a realm. P1 high-priority

Comments

@gnprice
Copy link
Member

gnprice commented Mar 10, 2020

This is for the client side, on iOS, of zulip/zulip#14168.

In addition to the links there, here are Apple docs specific to iOS:

Here's an implementation for RN which we may be able to use:

Note that although it's for React Native, it only works on iOS. It's a thin wrapper around the system-provided components.

We'll also want to support this on Android, because people may sign up using it on iOS and then also want to use Zulip on an Android device. That may have a separate implementation, so I'll file a separate issue for it.

@gnprice gnprice added a-iOS a-onboarding Everything you would do when first joining a realm. labels Mar 10, 2020
@gnprice gnprice added P1 high-priority blocked on other work To come back to after another related PR, or some other task. labels Mar 10, 2020
@mateuszmandera
Copy link

I think this one should be possible to work on without having anything done on the server yet? The first part of the native flow should be happening only between the app and Apple server - at the end of that the app should have a JWT token. All that's left at that point is to send the token with mobile_flow_otp to a server endpoint (probably /complete/apple-id) and the server will give the standard zulip:// redirect response - testing this part will require an implementation on the server.

@gnprice
Copy link
Member Author

gnprice commented Mar 19, 2020

Hmm -- yeah, after looking into this side of things some more, I agree. There's enough complexity involved before the point where we talk to the server that we can productively start working through that even while the server implementation is still being prepared.

OTOH, there is a different prerequisite this is blocked on! That handy existing implementation for RN requires something called react-native-unimodules, which in turn requires us to set up CocoaPods for our build. That's something we've been meaning to do, but will take some work to accomplish. I've just filed #3983 to give it an issue of its own; now that it's blocking this work, we should get on that soon.

So, thanks @mateuszmandera for poking us on this!

chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this issue Mar 20, 2020
There was an attempt to use CocoaPods in aded466, 2017-03-19.

That was removed in 4096e71, 2017-07-18.

Some cleanup was done in 861744b, 2020-02-05.

To paraphrase Greg on zulip#3983:

CocoaPods is a tool for managing iOS libraries. Many people use it
in their React Native apps, but there some tricky bits that have
impeded universal adoption.

RN v0.60 signals a full embrace of the use of CocoaPods by making it
work more smoothly with RN. CocoaPods becomes required in v0.61.

So, use CocoaPods, writing a Podfile based on the example Podfile
given for react-native v0.59, at
https://github.com/facebook/react-native-website/blob/ded79d2cf4456d8b1a4f67c2cdc1391789e70617/docs/integration-with-existing-apps.md.
(That doc isn't live on the react-native docs website because of
facebook/react-native-website#1603.)

Originally, we were going to start using CocoaPods as part of the RN
v0.60 upgrade, but we have a fresh reason to want to use CocoaPods,
with its own deadline: zulip#3964, Apple auth. Someone has made a handy
Expo package to handle Apple auth, but we can't use Expo packages
without either fully committing to using the Expo SDK (a dramatic
step) or using a package called react-native-unimodules, which lets
you select individual Expo packages. react-native-unimodules
requires the use of CocoaPods.
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this issue Mar 20, 2020
Following the parent, where we introduced Cocoapods, add
react-native-unimodules, using the example Podfile provided by that
package, at
https://gist.github.com/sjchmiela/6c079f2173938a9a61a7c6f053c45000.

react-native-unimodules lets us use individual packages that are
part of the Expo SDK. The one we want is expo-apple-authentication,
for zulip#3964.

Note that there is no caret (^) on 0.6.0! The maintainers have
assumed that if you're using the latest version of
react-native-unimodules, you're also using the latest version of
react-native. We encountered the error message posted at
https://github.com/unimodules/react-native-unimodules/issues/100,
and decided that staying with 0.6.0 is most feasible.

Support for react-native v0.59 dropped somewhere between 0.6.0 and
0.7.0-rc.4, so backwards compatibility (not to mention semantic
versioning) seems not to be a priority. 0.7.0-rc.4 will be important
for us, because that's when AndroidX compatibility is introduced,
according to
https://github.com/unimodules/react-native-unimodules/issues/100#issuecomment-566436685.
This may mean that AndroidX (open PR zulip#3852) and RN v0.60.0 (issue
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this issue Mar 20, 2020
There was an attempt to use CocoaPods in aded466, 2017-03-19.

That was removed in 4096e71, 2017-07-18.

Some cleanup was done in 861744b, 2020-02-05.

To paraphrase Greg on zulip#3983:

CocoaPods is a tool for managing iOS libraries. Many people use it
in their React Native apps, but there some tricky bits that have
impeded universal adoption.

RN v0.60 signals a full embrace of the use of CocoaPods by making it
work more smoothly with RN. CocoaPods becomes required in v0.61.

So, use CocoaPods, writing a Podfile based on the example Podfile
given for react-native v0.59, at
https://github.com/facebook/react-native-website/blob/ded79d2cf4456d8b1a4f67c2cdc1391789e70617/docs/integration-with-existing-apps.md.
(That doc isn't live on the react-native docs website because of
facebook/react-native-website#1603.)

Originally, we were going to start using CocoaPods as part of the RN
v0.60 upgrade, but we have a fresh reason to want to use CocoaPods,
with its own deadline: zulip#3964, Apple auth. Someone has made a handy
Expo package to handle Apple auth, but we can't use Expo packages
without either fully committing to using the Expo SDK (a dramatic
step) or using a package called react-native-unimodules, which lets
you select individual Expo packages. react-native-unimodules
requires the use of CocoaPods.

Fixes: zulip#3983
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this issue Mar 20, 2020
Following the parent, where we introduced Cocoapods, add
react-native-unimodules, using the example Podfile provided by that
package, at
https://gist.github.com/sjchmiela/6c079f2173938a9a61a7c6f053c45000.

react-native-unimodules lets us use individual packages that are
part of the Expo SDK. The one we want is expo-apple-authentication,
for zulip#3964.

Note that there is no caret (^) on 0.6.0! The maintainers have
assumed that if you're using the latest version of
react-native-unimodules, you're also using the latest version of
react-native. We encountered the error message posted at
https://github.com/unimodules/react-native-unimodules/issues/100,
and decided that staying with 0.6.0 is most feasible.

Support for react-native v0.59 dropped somewhere between 0.6.0 and
0.7.0-rc.4, so backwards compatibility (not to mention semantic
versioning) seems not to be a priority. 0.7.0-rc.4 will be important
for us, because that's when AndroidX compatibility is introduced,
according to
https://github.com/unimodules/react-native-unimodules/issues/100#issuecomment-566436685.
This may mean that AndroidX compatibility (open PR zulip#3852) and using
RN v0.60.0 (issue zulip#3548) may have to be done simultaneously.
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this issue Mar 20, 2020
Following the parent, where we introduced Cocoapods, add
react-native-unimodules, using the example Podfile provided by that
package, at
https://gist.github.com/sjchmiela/6c079f2173938a9a61a7c6f053c45000.

react-native-unimodules lets us use individual packages that are
part of the Expo SDK. The one we want is expo-apple-authentication,
for zulip#3964.

Note that there is no caret (^) on 0.6.0! The maintainers have
assumed that if you're using the latest version of
react-native-unimodules, you're also using the latest version of
react-native. We encountered the error message posted at
https://github.com/unimodules/react-native-unimodules/issues/100,
and decided that staying with 0.6.0 is most feasible.

Support for react-native v0.59 dropped somewhere between 0.6.0 and
0.7.0-rc.4, so backwards compatibility (not to mention semantic
versioning) seems not to be a priority. 0.7.0-rc.4 will be important
for us, because that's when AndroidX compatibility is introduced,
according to
https://github.com/unimodules/react-native-unimodules/issues/100#issuecomment-566436685.
This may mean that AndroidX compatibility (open PR zulip#3852) and using
RN v0.60.0 (issue zulip#3548) may have to be done simultaneously.

Note: The following failure occurred with `tools/test deps`:

```
Package "ua-parser-js" wants ^0.7.18 and could get 0.7.21, but got 0.7.20

Found duplicate dependencies in yarn.lock which could be dedup'd.
Run:

  yarn yarn-deduplicate && yarn
```

The suggested command was run, and `tools/test deps` succeeded.
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this issue Mar 23, 2020
There was an attempt to use CocoaPods in aded466, 2017-03-19.

That was removed in 4096e71, 2017-07-18.

Some cleanup was done in 861744b, 2020-02-05.

To paraphrase Greg on zulip#3983:

CocoaPods is a tool for managing iOS libraries. Many people use it
in their React Native apps, but there some tricky bits that have
impeded universal adoption.

RN v0.60 signals a full embrace of the use of CocoaPods by making it
work more smoothly with RN. CocoaPods becomes required in v0.61.

So, use CocoaPods, writing a Podfile based on the example Podfile
given for react-native v0.59, at
https://github.com/facebook/react-native-website/blob/ded79d2cf4456d8b1a4f67c2cdc1391789e70617/docs/integration-with-existing-apps.md.
(That doc isn't live on the react-native docs website because of
facebook/react-native-website#1603.)

Originally, we were going to start using CocoaPods as part of the RN
v0.60 upgrade, but we have a fresh reason to want to use CocoaPods,
with its own deadline: zulip#3964, Apple auth. Someone has made a handy
Expo package to handle Apple auth, but we can't use Expo packages
without either fully committing to using the Expo SDK (a dramatic
step) or using a package called react-native-unimodules, which lets
you select individual Expo packages. react-native-unimodules
requires the use of CocoaPods.

Fixes: zulip#3983
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this issue Mar 23, 2020
Following the parent, where we introduced Cocoapods, add
react-native-unimodules, using the example Podfile provided by that
package, at
https://gist.github.com/sjchmiela/6c079f2173938a9a61a7c6f053c45000.

react-native-unimodules lets us use individual packages that are
part of the Expo SDK. The one we want is expo-apple-authentication,
for zulip#3964.

Note that there is no caret (^) on 0.6.0! The maintainers have
assumed that if you're using the latest version of
react-native-unimodules, you're also using the latest version of
react-native. We encountered the error message posted at
https://github.com/unimodules/react-native-unimodules/issues/100,
and decided that staying with 0.6.0 is most feasible.

Support for react-native v0.59 dropped somewhere between 0.6.0 and
0.7.0-rc.4, so backwards compatibility (not to mention semantic
versioning) seems not to be a priority. 0.7.0-rc.4 will be important
for us, because that's when AndroidX compatibility is introduced,
according to
https://github.com/unimodules/react-native-unimodules/issues/100#issuecomment-566436685.
This may mean that AndroidX compatibility (open PR zulip#3852) and using
RN v0.60.0 (issue zulip#3548) may have to be done simultaneously.

Note: The following failure occurred with `tools/test deps`:

```
Package "ua-parser-js" wants ^0.7.18 and could get 0.7.21, but got 0.7.20

Found duplicate dependencies in yarn.lock which could be dedup'd.
Run:

  yarn yarn-deduplicate && yarn
```

The suggested command was run, and `tools/test deps` succeeded.
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this issue Mar 28, 2020
There was an attempt to use CocoaPods in aded466, 2017-03-19.

That was removed in 4096e71, 2017-07-18.

Some cleanup was done in 861744b, 2020-02-05.

To paraphrase Greg on zulip#3983:

CocoaPods is a tool for managing iOS libraries. It means instead of
having an `.xcodeproj` file for every library, and manually
connecting them to your app's own `.xcodeproj`, a separate
`Pods.xcodeproj` is generated from a simple, declarative Podfile,
and all you have to do to update or add dependencies is edit the
Podfile and run `pod install`. Many people use it in their React
Native apps, but there some tricky bits that have impeded universal
adoption.

RN v0.60 signals a full embrace of the use of CocoaPods by RN, by
making it work more smoothly. CocoaPods becomes required in v0.61.

Originally, we were going to start using CocoaPods as part of the RN
v0.60 upgrade, but we have a fresh reason to want to use CocoaPods,
with its own deadline: zulip#3964, Apple auth. Someone has made a handy
Expo package to handle Apple auth, but we can't use Expo packages
without either fully committing to using the Expo SDK (a dramatic
step) or using a package called react-native-unimodules, which lets
you select individual Expo packages. react-native-unimodules
requires the use of CocoaPods.

So, use CocoaPods now. Note that the Podfile will have to be
different when we upgrade to RN v0.60.0; see the parent for details.

Unfortunately, this has to be one giant commit instead of multiple
commits. From experimentation, this seems to be the minimal set of
changes that doesn't break functionality, which makes sense
intuitively when changing entirely between management strategies for
(somewhat) complex dependencies. We were also prompted to try this
strategy from solution 3 at this SO post:
https://stackoverflow.com/questions/53312887/error-on-archiving-react-native-app-in-xcode-multiple-commands-produce-libyog/55328241#5532824
But Greg has the most plausible theory for the actual reason why
this is necessary:

"""
One possibility is something like: we were getting React, as a
whole, from the direct references in the Xcode config; and so that
version of the built React library had versions of RCTImageView etc.
only when those were configured through RCTImage.xcodeproj etc., via
direct references to those in our Xcode config.

Whereas the built React library from React.podfile and our Podspec
had the RCTImageView that was built due to the RCTImage subspec in
our Podfile... but perhaps we just didn't get that one because it
had to compete with the libReact.a (or whatever) from our Xcode
config. And once we no longer had the latter, we started getting the
former instead.
"""

We started with a Podfile based on the example
Podfile given for react-native v0.59, at
https://github.com/facebook/react-native-website/blob/ded79d2cf4456d8b1a4f67c2cdc1391789e70617/docs/integration-with-existing-apps.md.
(That doc isn't live on the react-native docs website because of
facebook/react-native-website#1603.)

Then, we realized that more RN-provided libraries were present in
our Xcode config, so we added those.

We also added a number of dependencies that depend on React Native
(react-native-image-picker, etc.).
`grep -Rlw 'React' --include=\*.podspec node_modules/` verified
that they do indeed depend on the 'React' pod, which is React
Native.

Also, add a build phase to start Metro. For some reason that we
haven't found yet, Metro doesn't start automatically after the
switch to CocoaPods. It seems that this was recognized by the RN
maintainers as they were updating the template app to switch to
CocoaPods. They separated the addition of this build phase into its
own commit,
facebook/react-native@1f719ae43,
but it doesn't mention the reason it was added.

In that commit, the name "Start Packager" was used. Here, we use
"Start Metro" because a proper noun is more helpful, and this name
is what RN uses in a more recent commit,
facebook/react-native@e636eb60d, which
creates a RNTesterPods project and workspace for testing some
aspects of CocoaPods separately. (It's not clear without further
digging what aspects those are, but it hasn't been relevant to using
CocoaPods on RN v0.59.10.)

Notes on individual dependencies:

RNDeviceInfo (react-native-device-info):

The install instructions at
https://github.com/react-native-community/react-native-device-info#manual
for RN v0.59.0 using CocoaPods wrongly say not to use CocoaPods (by
omitting the line in the Podfile). The author's comment at
react-native-device-info/react-native-device-info#748 (comment)
suggests that he never found the successful strategy of doing an
all-at-once adoption of CocoaPods, as we do here, and rather
concluded that a bug was caused by RN v0.59.x having shaky support
for CocoaPods. Including the pod works fine.

Fixes: zulip#3983
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this issue Mar 30, 2020
There was an attempt to use CocoaPods in aded466, 2017-03-19.

That was removed in 4096e71, 2017-07-18.

Some cleanup was done in 861744b, 2020-02-05.

To paraphrase Greg on zulip#3983:

CocoaPods is a tool for managing iOS libraries. It means instead of
having an `.xcodeproj` file for every library, and manually
connecting them to your app's own `.xcodeproj`, a separate
`Pods.xcodeproj` is generated from a simple, declarative Podfile,
and all you have to do to update or add dependencies is edit the
Podfile and run `pod install`. Many people use it in their React
Native apps, but there some tricky bits that have impeded universal
adoption.

RN v0.60 signals a full embrace of the use of CocoaPods by RN, by
making it work more smoothly. CocoaPods becomes required in v0.61.

Originally, we were going to start using CocoaPods as part of the RN
v0.60 upgrade, but we have a fresh reason to want to use CocoaPods,
with its own deadline: zulip#3964, Apple auth. Someone has made a handy
Expo package to handle Apple auth, but we can't use Expo packages
without either fully committing to using the Expo SDK (a dramatic
step) or using a package called react-native-unimodules, which lets
you select individual Expo packages. react-native-unimodules
requires the use of CocoaPods.

So, use CocoaPods now. Note that the Podfile will have to be
different when we upgrade to RN v0.60.0; see the parent for details.

Unfortunately, this has to be one giant commit instead of multiple
commits. From experimentation, this seems to be the minimal set of
changes that doesn't break functionality, which makes sense
intuitively when changing entirely between management strategies for
(somewhat) complex dependencies. We were also prompted to try this
strategy from solution 3 at this SO post:
https://stackoverflow.com/questions/53312887/error-on-archiving-react-native-app-in-xcode-multiple-commands-produce-libyog/55328241#5532824
But Greg has the most plausible theory for the actual reason why
this is necessary:

"""
One possibility is something like: we were getting React, as a
whole, from the direct references in the Xcode config; and so that
version of the built React library had versions of RCTImageView etc.
only when those were configured through RCTImage.xcodeproj etc., via
direct references to those in our Xcode config.

Whereas the built React library from React.podfile and our Podspec
had the RCTImageView that was built due to the RCTImage subspec in
our Podfile... but perhaps we just didn't get that one because it
had to compete with the libReact.a (or whatever) from our Xcode
config. And once we no longer had the latter, we started getting the
former instead.
"""

We started with a Podfile based on the example
Podfile given for react-native v0.59, at
https://github.com/facebook/react-native-website/blob/ded79d2cf4456d8b1a4f67c2cdc1391789e70617/docs/integration-with-existing-apps.md.
(That doc isn't live on the react-native docs website because of
facebook/react-native-website#1603.)

Then, we realized that more RN-provided libraries were present in
our Xcode config, so we added those.

We also added a number of dependencies that depend on React Native
(react-native-image-picker, etc.).
`grep -Rlw 'React' --include=\*.podspec node_modules/` verified
that they do indeed depend on the 'React' pod, which is React
Native.

Also, add a build phase to start Metro. For some reason that we
haven't found yet, Metro doesn't start automatically after the
switch to CocoaPods. It seems that this was recognized by the RN
maintainers as they were updating the template app to switch to
CocoaPods. They separated the addition of this build phase into its
own commit,
facebook/react-native@1f719ae43,
but it doesn't mention the reason it was added.

In that commit, the name "Start Packager" was used. Here, we use
"Start Metro" because a proper noun is more helpful, and this name
is what RN uses in a more recent commit,
facebook/react-native@e636eb60d, which
creates a RNTesterPods project and workspace for testing some
aspects of CocoaPods separately. (It's not clear without further
digging what aspects those are, but it hasn't been relevant to using
CocoaPods on RN v0.59.10.)

Notes on individual dependencies:

RNDeviceInfo (react-native-device-info):

The install instructions at
https://github.com/react-native-community/react-native-device-info#manual
for RN v0.59.0 using CocoaPods wrongly say not to use CocoaPods (by
omitting the line in the Podfile). The author's comment at
react-native-device-info/react-native-device-info#748 (comment)
suggests that he never found the successful strategy of doing an
all-at-once adoption of CocoaPods, as we do here, and rather
concluded that a bug was caused by RN v0.59.x having shaky support
for CocoaPods. Including the pod works fine.

Fixes: zulip#3983
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this issue Apr 1, 2020
There was an attempt to use CocoaPods in aded466, 2017-03-19.

That was removed in 4096e71, 2017-07-18.

Some cleanup was done in 861744b, 2020-02-05.

To paraphrase Greg on zulip#3983:

CocoaPods is a tool for managing iOS libraries. It means instead of
having an `.xcodeproj` file for every library, and manually
connecting them to your app's own `.xcodeproj`, a separate
`Pods.xcodeproj` is generated from a simple, declarative Podfile,
and all you have to do to update or add dependencies is edit the
Podfile and run `pod install`. Many people use it in their React
Native apps, but there some tricky bits that have impeded universal
adoption.

RN v0.60 signals a full embrace of the use of CocoaPods by RN, by
making it work more smoothly. CocoaPods becomes required in v0.61.

Originally, we were going to start using CocoaPods as part of the RN
v0.60 upgrade, but we have a fresh reason to want to use CocoaPods,
with its own deadline: zulip#3964, Apple auth. Someone has made a handy
Expo package to handle Apple auth, but we can't use Expo packages
without either fully committing to using the Expo SDK (a dramatic
step) or using a package called react-native-unimodules, which lets
you select individual Expo packages. react-native-unimodules
requires the use of CocoaPods.

So, use CocoaPods now. Note that the Podfile will have to be
different when we upgrade to RN v0.60.0; see the parent for details.

Unfortunately, this has to be one giant commit instead of multiple
commits. From experimentation, this seems to be the minimal set of
changes that doesn't break functionality, which makes sense
intuitively when changing entirely between management strategies for
(somewhat) complex dependencies. We were also prompted to try this
strategy from solution 3 at this SO post:
https://stackoverflow.com/questions/53312887/error-on-archiving-react-native-app-in-xcode-multiple-commands-produce-libyog/55328241#5532824
But Greg has the most plausible theory for the actual reason why
this is necessary:

"""
One possibility is something like: we were getting React, as a
whole, from the direct references in the Xcode config; and so that
version of the built React library had versions of RCTImageView etc.
only when those were configured through RCTImage.xcodeproj etc., via
direct references to those in our Xcode config.

Whereas the built React library from React.podfile and our Podspec
had the RCTImageView that was built due to the RCTImage subspec in
our Podfile... but perhaps we just didn't get that one because it
had to compete with the libReact.a (or whatever) from our Xcode
config. And once we no longer had the latter, we started getting the
former instead.
"""

We started with a Podfile based on the example
Podfile given for react-native v0.59, at
https://github.com/facebook/react-native-website/blob/ded79d2cf4456d8b1a4f67c2cdc1391789e70617/docs/integration-with-existing-apps.md.
(That doc isn't live on the react-native docs website because of
facebook/react-native-website#1603.)

Then, we realized that more RN-provided libraries were present in
our Xcode config, so we added those.

We also added a number of dependencies that depend on React Native
(react-native-image-picker, etc.).
`grep -Rlw 'React' --include=\*.podspec node_modules/` verified
that they do indeed depend on the 'React' pod, which is React
Native.

Also, add a build phase to start Metro. For some reason that we
haven't found yet, Metro doesn't start automatically after the
switch to CocoaPods. It seems that this was recognized by the RN
maintainers as they were updating the template app to switch to
CocoaPods. They separated the addition of this build phase into its
own commit,
facebook/react-native@1f719ae43,
but it doesn't mention the reason it was added.

In that commit, the name "Start Packager" was used. Here, we use
"Start Metro" because a proper noun is more helpful, and this name
is what RN uses in a more recent commit,
facebook/react-native@e636eb60d, which
creates a RNTesterPods project and workspace for testing some
aspects of CocoaPods separately. (It's not clear without further
digging what aspects those are, but it hasn't been relevant to using
CocoaPods on RN v0.59.10.)

Notes on individual dependencies:

RNDeviceInfo (react-native-device-info):

The install instructions at
https://github.com/react-native-community/react-native-device-info#manual
for RN v0.59.0 using CocoaPods wrongly say not to use CocoaPods (by
omitting the line in the Podfile). The author's comment at
react-native-device-info/react-native-device-info#748 (comment)
suggests that he never found the successful strategy of doing an
all-at-once adoption of CocoaPods, as we do here, and rather
concluded that a bug was caused by RN v0.59.x having shaky support
for CocoaPods. Including the pod works fine.

Fixes: zulip#3983
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this issue Apr 1, 2020
There was an attempt to use CocoaPods in aded466, 2017-03-19.

That was removed in 4096e71, 2017-07-18.

Some cleanup was done in 861744b, 2020-02-05.

To paraphrase Greg on zulip#3983:

CocoaPods is a tool for managing iOS libraries. It means instead of
having an `.xcodeproj` file for every library, and manually
connecting them to your app's own `.xcodeproj`, a separate
`Pods.xcodeproj` is generated from a simple, declarative Podfile,
and all you have to do to update or add dependencies is edit the
Podfile and run `pod install`. Many people use it in their React
Native apps, but there some tricky bits that have impeded universal
adoption.

RN v0.60 signals a full embrace of the use of CocoaPods by RN, by
making it work more smoothly. CocoaPods becomes required in v0.61.

Originally, we were going to start using CocoaPods as part of the RN
v0.60 upgrade, but we have a fresh reason to want to use CocoaPods,
with its own deadline: zulip#3964, Apple auth. Someone has made a handy
Expo package to handle Apple auth, but we can't use Expo packages
without either fully committing to using the Expo SDK (a dramatic
step) or using a package called react-native-unimodules, which lets
you select individual Expo packages. react-native-unimodules
requires the use of CocoaPods.

So, use CocoaPods now. Note that the Podfile will have to be
different when we upgrade to RN v0.60.0; see the parent for details.

Unfortunately, this has to be one giant commit instead of multiple
commits. From experimentation, this seems to be the minimal set of
changes that doesn't break functionality, which makes sense
intuitively when changing entirely between management strategies for
(somewhat) complex dependencies. We were also prompted to try this
strategy from solution 3 at this SO post:
https://stackoverflow.com/questions/53312887/error-on-archiving-react-native-app-in-xcode-multiple-commands-produce-libyog/55328241#5532824
But Greg has the most plausible theory for the actual reason why
this is necessary:

"""
One possibility is something like: we were getting React, as a
whole, from the direct references in the Xcode config; and so that
version of the built React library had versions of RCTImageView etc.
only when those were configured through RCTImage.xcodeproj etc., via
direct references to those in our Xcode config.

Whereas the built React library from React.podfile and our Podspec
had the RCTImageView that was built due to the RCTImage subspec in
our Podfile... but perhaps we just didn't get that one because it
had to compete with the libReact.a (or whatever) from our Xcode
config. And once we no longer had the latter, we started getting the
former instead.
"""

We started with a Podfile based on the example
Podfile given for react-native v0.59, at
https://github.com/facebook/react-native-website/blob/ded79d2cf4456d8b1a4f67c2cdc1391789e70617/docs/integration-with-existing-apps.md.
(That doc isn't live on the react-native docs website because of
facebook/react-native-website#1603.)

Then, we realized that more RN-provided libraries were present in
our Xcode config, so we added those.

We also added a number of dependencies that depend on React Native
(react-native-image-picker, etc.).
`grep -Rlw 'React' --include=\*.podspec node_modules/` verified
that they do indeed depend on the 'React' pod, which is React
Native.

Also, add a build phase to start Metro. For some reason that we
haven't found yet, Metro doesn't start automatically after the
switch to CocoaPods. It seems that this was recognized by the RN
maintainers as they were updating the template app to switch to
CocoaPods. They separated the addition of this build phase into its
own commit,
facebook/react-native@1f719ae43,
but it doesn't mention the reason it was added.

In that commit, the name "Start Packager" was used. Here, we use
"Start Metro" because a proper noun is more helpful, and this name
is what RN uses in a more recent commit,
facebook/react-native@e636eb60d, which
creates a RNTesterPods project and workspace for testing some
aspects of CocoaPods separately. (It's not clear without further
digging what aspects those are, but it hasn't been relevant to using
CocoaPods on RN v0.59.10.)

Notes on individual dependencies:

RNDeviceInfo (react-native-device-info):

The install instructions at
https://github.com/react-native-community/react-native-device-info#manual
for RN v0.59.0 using CocoaPods wrongly say not to use CocoaPods (by
omitting the line in the Podfile). The author's comment at
react-native-device-info/react-native-device-info#748 (comment)
suggests that he never found the successful strategy of doing an
all-at-once adoption of CocoaPods, as we do here, and rather
concluded that a bug was caused by RN v0.59.x having shaky support
for CocoaPods. Including the pod works fine.

Fixes: zulip#3983
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this issue Apr 1, 2020
There was an attempt to use CocoaPods in aded466, 2017-03-19.

That was removed in 4096e71, 2017-07-18.

Some cleanup was done in 861744b, 2020-02-05.

To paraphrase Greg on zulip#3983:

CocoaPods is a tool for managing iOS libraries. It means instead of
having an `.xcodeproj` file for every library, and manually
connecting them to your app's own `.xcodeproj`, a separate
`Pods.xcodeproj` is generated from a simple, declarative Podfile,
and all you have to do to update or add dependencies is edit the
Podfile and run `pod install`. Many people use it in their React
Native apps, but there some tricky bits that have impeded universal
adoption.

RN v0.60 signals a full embrace of the use of CocoaPods by RN, by
making it work more smoothly. CocoaPods becomes required in v0.61.

Originally, we were going to start using CocoaPods as part of the RN
v0.60 upgrade, but we have a fresh reason to want to use CocoaPods,
with its own deadline: zulip#3964, Apple auth. Someone has made a handy
Expo package to handle Apple auth, but we can't use Expo packages
without either fully committing to using the Expo SDK (a dramatic
step) or using a package called react-native-unimodules, which lets
you select individual Expo packages. react-native-unimodules
requires the use of CocoaPods.

So, use CocoaPods now. Note that the Podfile will have to be
different when we upgrade to RN v0.60.0; see the parent for details.

Unfortunately, this has to be one giant commit instead of multiple
commits. From experimentation, this seems to be the minimal set of
changes that doesn't break functionality, which makes sense
intuitively when changing entirely between management strategies for
(somewhat) complex dependencies. We were also prompted to try this
strategy from solution 3 at this SO post:
https://stackoverflow.com/questions/53312887/error-on-archiving-react-native-app-in-xcode-multiple-commands-produce-libyog/55328241#5532824
But Greg has the most plausible theory for the actual reason why
this is necessary:

"""
One possibility is something like: we were getting React, as a
whole, from the direct references in the Xcode config; and so that
version of the built React library had versions of RCTImageView etc.
only when those were configured through RCTImage.xcodeproj etc., via
direct references to those in our Xcode config.

Whereas the built React library from React.podfile and our Podspec
had the RCTImageView that was built due to the RCTImage subspec in
our Podfile... but perhaps we just didn't get that one because it
had to compete with the libReact.a (or whatever) from our Xcode
config. And once we no longer had the latter, we started getting the
former instead.
"""

We started with a Podfile based on the example
Podfile given for react-native v0.59, at
https://github.com/facebook/react-native-website/blob/ded79d2cf4456d8b1a4f67c2cdc1391789e70617/docs/integration-with-existing-apps.md.
(That doc isn't live on the react-native docs website because of
facebook/react-native-website#1603.)

Then, we realized that more RN-provided libraries were present in
our Xcode config, so we added those.

We also added a number of dependencies that depend on React Native
(react-native-image-picker, etc.).
`grep -Rlw 'React' --include=\*.podspec node_modules/` verified
that they do indeed depend on the 'React' pod, which is React
Native.

Also, add a build phase to start Metro. For some reason that we
haven't found yet, Metro doesn't start automatically after the
switch to CocoaPods. It seems that this was recognized by the RN
maintainers as they were updating the template app to switch to
CocoaPods. They separated the addition of this build phase into its
own commit,
facebook/react-native@1f719ae43,
but it doesn't mention the reason it was added.

In that commit, the name "Start Packager" was used. Here, we use
"Start Metro" because a proper noun is more helpful, and this name
is what RN uses in a more recent commit,
facebook/react-native@e636eb60d, which
creates a RNTesterPods project and workspace for testing some
aspects of CocoaPods separately. (It's not clear without further
digging what aspects those are, but it hasn't been relevant to using
CocoaPods on RN v0.59.10.)

Notes on individual dependencies:

RNDeviceInfo (react-native-device-info):

The install instructions at
https://github.com/react-native-community/react-native-device-info#manual
for RN v0.59.0 using CocoaPods wrongly say not to use CocoaPods (by
omitting the line in the Podfile). The author's comment at
react-native-device-info/react-native-device-info#748 (comment)
suggests that he never found the successful strategy of doing an
all-at-once adoption of CocoaPods, as we do here, and rather
concluded that a bug was caused by RN v0.59.x having shaky support
for CocoaPods. Including the pod works fine.

Fixes: zulip#3983
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this issue Apr 2, 2020
There was an attempt to use CocoaPods in aded466, 2017-03-19.

That was removed in 4096e71, 2017-07-18.

Some cleanup was done in 861744b, 2020-02-05.

To paraphrase Greg on zulip#3983:

CocoaPods is a tool for managing iOS libraries. It means instead of
having an `.xcodeproj` file for every library, and manually
connecting them to your app's own `.xcodeproj`, a separate
`Pods.xcodeproj` is generated from a simple, declarative Podfile,
and all you have to do to update or add dependencies is edit the
Podfile and run `pod install`. Many people use it in their React
Native apps, but there some tricky bits that have impeded universal
adoption.

RN v0.60 signals a full embrace of the use of CocoaPods by RN, by
making it work more smoothly. CocoaPods becomes required in v0.61.

Originally, we were going to start using CocoaPods as part of the RN
v0.60 upgrade, but we have a fresh reason to want to use CocoaPods,
with its own deadline: zulip#3964, Apple auth. Someone has made a handy
Expo package to handle Apple auth, but we can't use Expo packages
without either fully committing to using the Expo SDK (a dramatic
step) or using a package called react-native-unimodules, which lets
you select individual Expo packages. react-native-unimodules
requires the use of CocoaPods.

So, use CocoaPods now. Note that the Podfile will have to be
different when we upgrade to RN v0.60.0; see the parent for details.

Unfortunately, this has to be one giant commit instead of multiple
commits. From experimentation, this seems to be the minimal set of
changes that doesn't break functionality, which makes sense
intuitively when changing entirely between management strategies for
(somewhat) complex dependencies. We were also prompted to try this
strategy from solution 3 at this SO post:
https://stackoverflow.com/questions/53312887/error-on-archiving-react-native-app-in-xcode-multiple-commands-produce-libyog/55328241#5532824
But Greg has the most plausible theory for the actual reason why
this is necessary:

"""
One possibility is something like: we were getting React, as a
whole, from the direct references in the Xcode config; and so that
version of the built React library had versions of RCTImageView etc.
only when those were configured through RCTImage.xcodeproj etc., via
direct references to those in our Xcode config.

Whereas the built React library from React.podfile and our Podspec
had the RCTImageView that was built due to the RCTImage subspec in
our Podfile... but perhaps we just didn't get that one because it
had to compete with the libReact.a (or whatever) from our Xcode
config. And once we no longer had the latter, we started getting the
former instead.
"""

We started with a Podfile based on the example
Podfile given for react-native v0.59, at
https://github.com/facebook/react-native-website/blob/ded79d2cf4456d8b1a4f67c2cdc1391789e70617/docs/integration-with-existing-apps.md.
(That doc isn't live on the react-native docs website because of
facebook/react-native-website#1603.)

Then, we realized that more RN-provided libraries were present in
our Xcode config, so we added those.

We also added a number of dependencies that depend on React Native
(react-native-image-picker, etc.).
`grep -Rlw 'React' --include=\*.podspec node_modules/` verified
that they do indeed depend on the 'React' pod, which is React
Native.

Also, add a build phase to start Metro. For some reason that we
haven't found yet, Metro doesn't start automatically after the
switch to CocoaPods. It seems that this was recognized by the RN
maintainers as they were updating the template app to switch to
CocoaPods. They separated the addition of this build phase into its
own commit,
facebook/react-native@1f719ae43,
but it doesn't mention the reason it was added.

In that commit, the name "Start Packager" was used. Here, we use
"Start Metro" because a proper noun is more helpful, and this name
is what RN uses in a more recent commit,
facebook/react-native@e636eb60d, which
creates a RNTesterPods project and workspace for testing some
aspects of CocoaPods separately. (It's not clear without further
digging what aspects those are, but it hasn't been relevant to using
CocoaPods on RN v0.59.10.)

Notes on individual dependencies:

RNDeviceInfo (react-native-device-info):

The install instructions at
https://github.com/react-native-community/react-native-device-info#manual
for RN v0.59.0 using CocoaPods wrongly say not to use CocoaPods (by
omitting the line in the Podfile). The author's comment at
react-native-device-info/react-native-device-info#748 (comment)
suggests that he never found the successful strategy of doing an
all-at-once adoption of CocoaPods, as we do here, and rather
concluded that a bug was caused by RN v0.59.x having shaky support
for CocoaPods. Including the pod works fine.

Fixes: zulip#3983
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this issue Apr 2, 2020
There was an attempt to use CocoaPods in aded466, 2017-03-19.

That was removed in 4096e71, 2017-07-18.

Some cleanup was done in 861744b, 2020-02-05.

To paraphrase Greg on zulip#3983:

CocoaPods is a tool for managing iOS libraries. It means instead of
having an `.xcodeproj` file for every library, and manually
connecting them to your app's own `.xcodeproj`, a separate
`Pods.xcodeproj` is generated from a simple, declarative Podfile,
and all you have to do to update or add dependencies is edit the
Podfile and run `pod install`. Many people use it in their React
Native apps, but there some tricky bits that have impeded universal
adoption.

RN v0.60 signals a full embrace of the use of CocoaPods by RN, by
making it work more smoothly. CocoaPods becomes required in v0.61.

Originally, we were going to start using CocoaPods as part of the RN
v0.60 upgrade, but we have a fresh reason to want to use CocoaPods,
with its own deadline: zulip#3964, Apple auth. Someone has made a handy
Expo package to handle Apple auth, but we can't use Expo packages
without either fully committing to using the Expo SDK (a dramatic
step) or using a package called react-native-unimodules, which lets
you select individual Expo packages. react-native-unimodules
requires the use of CocoaPods.

So, use CocoaPods now. Note that the Podfile will have to be
different when we upgrade to RN v0.60.0; see the parent for details.

Unfortunately, this has to be one giant commit instead of multiple
commits. From experimentation, this seems to be the minimal set of
changes that doesn't break functionality, which makes sense
intuitively when changing entirely between management strategies for
(somewhat) complex dependencies. We were also prompted to try this
strategy from solution 3 at this SO post:
https://stackoverflow.com/questions/53312887/error-on-archiving-react-native-app-in-xcode-multiple-commands-produce-libyog/55328241#5532824
But Greg has the most plausible theory for the actual reason why
this is necessary:

"""
One possibility is something like: we were getting React, as a
whole, from the direct references in the Xcode config; and so that
version of the built React library had versions of RCTImageView etc.
only when those were configured through RCTImage.xcodeproj etc., via
direct references to those in our Xcode config.

Whereas the built React library from React.podfile and our Podspec
had the RCTImageView that was built due to the RCTImage subspec in
our Podfile... but perhaps we just didn't get that one because it
had to compete with the libReact.a (or whatever) from our Xcode
config. And once we no longer had the latter, we started getting the
former instead.
"""

We started with a Podfile based on the example
Podfile given for react-native v0.59, at
https://github.com/facebook/react-native-website/blob/ded79d2cf4456d8b1a4f67c2cdc1391789e70617/docs/integration-with-existing-apps.md.
(That doc isn't live on the react-native docs website because of
facebook/react-native-website#1603.)

Then, we realized that more RN-provided libraries were present in
our Xcode config, so we added those.

We also added a number of dependencies that depend on React Native
(react-native-image-picker, etc.).
`grep -Rlw 'React' --include=\*.podspec node_modules/` verified
that they do indeed depend on the 'React' pod, which is React
Native.

Also, add a build phase to start Metro. For some reason that we
haven't found yet, Metro doesn't start automatically after the
switch to CocoaPods. It seems that this was recognized by the RN
maintainers as they were updating the template app to switch to
CocoaPods. They separated the addition of this build phase into its
own commit,
facebook/react-native@1f719ae43,
but it doesn't mention the reason it was added.

In that commit, the name "Start Packager" was used. Here, we use
"Start Metro" because a proper noun is more helpful, and this name
is what RN uses in a more recent commit,
facebook/react-native@e636eb60d, which
creates a RNTesterPods project and workspace for testing some
aspects of CocoaPods separately. (It's not clear without further
digging what aspects those are, but it hasn't been relevant to using
CocoaPods on RN v0.59.10.)

Notes on individual dependencies:

RNDeviceInfo (react-native-device-info):

The install instructions at
https://github.com/react-native-community/react-native-device-info#manual
for RN v0.59.0 using CocoaPods wrongly say not to use CocoaPods (by
omitting the line in the Podfile). The author's comment at
react-native-device-info/react-native-device-info#748 (comment)
suggests that he never found the successful strategy of doing an
all-at-once adoption of CocoaPods, as we do here, and rather
concluded that a bug was caused by RN v0.59.x having shaky support
for CocoaPods. Including the pod works fine.

Fixes: zulip#3983
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this issue Apr 2, 2020
There was an attempt to use CocoaPods in aded466, 2017-03-19.

That was removed in 4096e71, 2017-07-18.

Some cleanup was done in 861744b, 2020-02-05.

To paraphrase Greg on zulip#3983:

CocoaPods is a tool for managing iOS libraries. It means instead of
having an `.xcodeproj` file for every library, and manually
connecting them to your app's own `.xcodeproj`, a separate
`Pods.xcodeproj` is generated from a simple, declarative Podfile,
and all you have to do to update or add dependencies is edit the
Podfile and run `pod install`. Many people use it in their React
Native apps, but there some tricky bits that have impeded universal
adoption.

RN v0.60 signals a full embrace of the use of CocoaPods by RN, by
making it work more smoothly. CocoaPods becomes required in v0.61.

Originally, we were going to start using CocoaPods as part of the RN
v0.60 upgrade, but we have a fresh reason to want to use CocoaPods,
with its own deadline: zulip#3964, Apple auth. Someone has made a handy
Expo package to handle Apple auth, but we can't use Expo packages
without either fully committing to using the Expo SDK (a dramatic
step) or using a package called react-native-unimodules, which lets
you select individual Expo packages. react-native-unimodules
requires the use of CocoaPods.

So, use CocoaPods now. Note that the Podfile will have to be
different when we upgrade to RN v0.60.0; see the parent for details.

Unfortunately, this has to be one giant commit instead of multiple
commits. From experimentation, this seems to be the minimal set of
changes that doesn't break functionality, which makes sense
intuitively when changing entirely between management strategies for
(somewhat) complex dependencies. We were also prompted to try this
strategy from solution 3 at this SO post:
https://stackoverflow.com/questions/53312887/error-on-archiving-react-native-app-in-xcode-multiple-commands-produce-libyog/55328241#5532824
But Greg has the most plausible theory for the actual reason why
this is necessary:

"""
One possibility is something like: we were getting React, as a
whole, from the direct references in the Xcode config; and so that
version of the built React library had versions of RCTImageView etc.
only when those were configured through RCTImage.xcodeproj etc., via
direct references to those in our Xcode config.

Whereas the built React library from React.podfile and our Podspec
had the RCTImageView that was built due to the RCTImage subspec in
our Podfile... but perhaps we just didn't get that one because it
had to compete with the libReact.a (or whatever) from our Xcode
config. And once we no longer had the latter, we started getting the
former instead.
"""

We started with a Podfile based on the example
Podfile given for react-native v0.59, at
https://github.com/facebook/react-native-website/blob/ded79d2cf4456d8b1a4f67c2cdc1391789e70617/docs/integration-with-existing-apps.md.
(That doc isn't live on the react-native docs website because of
facebook/react-native-website#1603.)

Then, we realized that more RN-provided libraries were present in
our Xcode config, so we added those.

We also added a number of dependencies that depend on React Native
(react-native-image-picker, etc.).
`grep -Rlw 'React' --include=\*.podspec node_modules/` verified
that they do indeed depend on the 'React' pod, which is React
Native.

Also, add a build phase to start Metro. For some reason that we
haven't found yet, Metro doesn't start automatically after the
switch to CocoaPods. It seems that this was recognized by the RN
maintainers as they were updating the template app to switch to
CocoaPods. They separated the addition of this build phase into its
own commit,
facebook/react-native@1f719ae43,
but it doesn't mention the reason it was added.

In that commit, the name "Start Packager" was used. Here, we use
"Start Metro" because a proper noun is more helpful, and this name
is what RN uses in a more recent commit,
facebook/react-native@e636eb60d, which
creates a RNTesterPods project and workspace for testing some
aspects of CocoaPods separately. (It's not clear without further
digging what aspects those are, but it hasn't been relevant to using
CocoaPods on RN v0.59.10.)

Notes on individual dependencies:

RNDeviceInfo (react-native-device-info):

The install instructions at
https://github.com/react-native-community/react-native-device-info#manual
for RN v0.59.0 using CocoaPods wrongly say not to use CocoaPods (by
omitting the line in the Podfile). The author's comment at
react-native-device-info/react-native-device-info#748 (comment)
suggests that he never found the successful strategy of doing an
all-at-once adoption of CocoaPods, as we do here, and rather
concluded that a bug was caused by RN v0.59.x having shaky support
for CocoaPods. Including the pod works fine.

Fixes: zulip#3983
@chrisbobbe chrisbobbe self-assigned this Apr 3, 2020
@chrisbobbe
Copy link
Contributor

@gnprice, when you get a chance, there are some necessary Apple Developer Console actions that you're authorized to take. From Expo's AppleAuthentication doc (link to "Configuration" section):

  1. Log into the Apple Developer Console, go to "Certificates, Identifiers, & Profiles" and then "Identifiers".
    [...]
  2. In the list of identifiers, click on the one corresponding to your primary app. Enable the "Sign In with Apple" capability, click "Edit", and choose the "Enable as a primary App ID" option. Save the new configuration.
    [...]
  3. Next, go to the "Keys" page and register a new key. Add the "Sign In with Apple" capability, and make sure to choose the correct primary app on the configuration screen.

gnprice pushed a commit to chrisbobbe/zulip-mobile that referenced this issue Apr 3, 2020
There was an attempt to use CocoaPods in aded466, 2017-03-19.

That was removed in 4096e71, 2017-07-18.

Some cleanup was done in 861744b, 2020-02-05.

To paraphrase Greg on zulip#3983:

CocoaPods is a tool for managing iOS libraries. It means instead of
having an `.xcodeproj` file for every library, and manually
connecting them to your app's own `.xcodeproj`, a separate
`Pods.xcodeproj` is generated from a simple, declarative Podfile,
and all you have to do to update or add dependencies is edit the
Podfile and run `pod install`. Many people use it in their React
Native apps, but there some tricky bits that have impeded universal
adoption.

RN v0.60 signals a full embrace of the use of CocoaPods by RN, by
making it work more smoothly. CocoaPods becomes required in v0.61.

Originally, we were going to start using CocoaPods as part of the RN
v0.60 upgrade, but we have a fresh reason to want to use CocoaPods,
with its own deadline: zulip#3964, Apple auth. Someone has made a handy
Expo package to handle Apple auth, but we can't use Expo packages
without either fully committing to using the Expo SDK (a dramatic
step) or using a package called react-native-unimodules, which lets
you select individual Expo packages. react-native-unimodules
requires the use of CocoaPods.

So, use CocoaPods now. Note that the Podfile will have to be
different when we upgrade to RN v0.60.0; see the parent for details.

Unfortunately, this has to be one giant commit instead of multiple
commits. From experimentation, this seems to be the minimal set of
changes that doesn't break functionality, which makes sense
intuitively when changing entirely between management strategies for
(somewhat) complex dependencies. We were also prompted to try this
strategy from solution 3 at this SO post:
https://stackoverflow.com/questions/53312887/error-on-archiving-react-native-app-in-xcode-multiple-commands-produce-libyog/55328241#5532824
But Greg has the most plausible theory for the actual reason why
this is necessary:

"""
One possibility is something like: we were getting React, as a
whole, from the direct references in the Xcode config; and so that
version of the built React library had versions of RCTImageView etc.
only when those were configured through RCTImage.xcodeproj etc., via
direct references to those in our Xcode config.

Whereas the built React library from React.podfile and our Podspec
had the RCTImageView that was built due to the RCTImage subspec in
our Podfile... but perhaps we just didn't get that one because it
had to compete with the libReact.a (or whatever) from our Xcode
config. And once we no longer had the latter, we started getting the
former instead.
"""

We started with a Podfile based on the example
Podfile given for react-native v0.59, at
https://github.com/facebook/react-native-website/blob/ded79d2cf4456d8b1a4f67c2cdc1391789e70617/docs/integration-with-existing-apps.md.
(That doc isn't live on the react-native docs website because of
facebook/react-native-website#1603.)

Then, we realized that more RN-provided libraries were present in
our Xcode config, so we added those.

We also added a number of dependencies that depend on React Native
(react-native-image-picker, etc.).
`grep -Rlw 'React' --include=\*.podspec node_modules/` verified
that they do indeed depend on the 'React' pod, which is React
Native.

Also, add a build phase to start Metro. For some reason that we
haven't found yet, Metro doesn't start automatically after the
switch to CocoaPods. It seems that this was recognized by the RN
maintainers as they were updating the template app to switch to
CocoaPods. They separated the addition of this build phase into its
own commit,
facebook/react-native@1f719ae43,
but it doesn't mention the reason it was added.

In that commit, the name "Start Packager" was used. Here, we use
"Start Metro" because a proper noun is more helpful, and this name
is what RN uses in a more recent commit,
facebook/react-native@e636eb60d, which
creates a RNTesterPods project and workspace for testing some
aspects of CocoaPods separately. (It's not clear without further
digging what aspects those are, but it hasn't been relevant to using
CocoaPods on RN v0.59.10.)

Notes on individual dependencies:

RNDeviceInfo (react-native-device-info):

The install instructions at
https://github.com/react-native-community/react-native-device-info#manual
for RN v0.59.0 using CocoaPods wrongly say not to use CocoaPods (by
omitting the line in the Podfile). The author's comment at
react-native-device-info/react-native-device-info#748 (comment)
suggests that he never found the successful strategy of doing an
all-at-once adoption of CocoaPods, as we do here, and rather
concluded that a bug was caused by RN v0.59.x having shaky support
for CocoaPods. Including the pod works fine.

Fixes: zulip#3983
@gnprice
Copy link
Member Author

gnprice commented Apr 3, 2020

@gnprice, when you get a chance, there are some necessary Apple Developer Console actions that you're authorized to take.

Thanks -- done!

Let me know if you need the Key ID or other details and can't see them in the console yourself.

The key itself, I imagine will end up on our production servers to be actually used, as part of deploying zulip/zulip#14168. I've saved it locally, and also sent a copy to Tim (end-to-end encrypted) so there's a backup. (Note mostly to myself: I found this somewhat informative about what this key file is.)

@chrisbobbe
Copy link
Contributor

chrisbobbe commented Apr 4, 2020

OK, sounds good!

We do have some control over the appearance of the button, but not enough to make it look just like the other buttons. (Apple doc, Expo doc)

It looks like we have full control over the corner radius, as long as we use a special prop, which is fine. In these screenshots, I've set it to 22, the same as the others. And we can set the height and width to match the other buttons.

At that point, we can set the following:

For buttonType:

  • SIGN_IN: The button's text (in English, anyway) is "Sign in with Apple".
  • CONTINUE: The button's text (in English, anyway) is "Continue with Apple". Here's one screenshot of CONTINUE:

image

For buttonStyle:

  • WHITE,
  • WHITE_OUTLINE
  • BLACK.

apple sign in button

@gnprice
Copy link
Member Author

gnprice commented Apr 4, 2020

Hmmm, interesting.

Those Expo docs say some things that aren't true, like:

The App Store Guidelines require you to use this component to start the authentication process instead of a custom button.

when here's Apple's guidelines on using a custom button:

If your layout requires it, you can create a custom Sign in with Apple button for iOS, macOS, or the web. For example, if you support multiple sign-in methods, you may want to display sign-in buttons that use left-aligned logos, or that display a logo only. [...]

That decreases my confidence somewhat in the accuracy of this Expo library's implementation. :-/

Still, surely going to be easiest at least for now to go along with that library's limitations, even if it means things look a little janky.

  • For where we say "Log in", the right text is SIGN_IN.
  • I think I prefer the button styles that look most like the other buttons.
    • A possible followup is to take this as a prompt to revisit the other buttons' designs too -- even apart from consistency with the Apple buttons, they can probably be improved.

@chrisbobbe
Copy link
Contributor

chrisbobbe commented Apr 4, 2020

Oh, interesting. From reading the Apple doc I linked to, I wouldn't have guessed that Apple lets you create a custom button:

Choose one of the built-in button styles and types, and change the corner radius of the button by setting the cornerRadius property, but don’t otherwise modify the style of the button.

It seems very Apple not to mention, on that crucial bit of documentation for what looks like The Way to do Apple auth, that some custom buttons are allowed. It's not quite as far as self-contradictory documentation, which we've seen with Apple before, since the ASAuthorizationAppleIDButton doc seems carefully worded to avoid categorically saying that this is the only approved button to use.

Without having gone through the entire implementation, I do think that the Expo doc seems reasonably accurate as far as it describes the UI constraints imposed by this particular Apple widget, which it uses. Apple's "Using the System-Provided Buttons" in the doc you linked to seems to correspond well with Expo's warnings and guidance.

I wonder if Expo ran into a lot of resistance from Apple on their submissions for review, as they were developing this (certainly reasonable, I'd say). That might explain Expo's laser-focus in their documentation on what Apple doesn't allow, as a good-willed effort to help developers avoid App store rejections. If this is the case, I might also expect that Expo's implementation is, in fact, optimized for accuracy — at least, accuracy in the correct use of this widget, and any other aspects of an Apple auth implementation that have tight published guidelines. It doesn't sound like it would need to be a very substantial wrapper, but I haven't looked very deeply.

Still, it would be most correct if Expo also mentioned that other UI choices are allowed (just that they introduce risk of rejection). I guess we could eventually use our own custom button instead of Expo's wrapper (probably starting from the custom button examples at the doc you linked to), and if we do, we'd probably still be able to rely on Expo for the non-graphical stuff (checking the status of credentials, actually signing in, etc.).

@chrisbobbe
Copy link
Contributor

I think I prefer the button styles that look most like the other buttons.

Maybe, the "WHITE_OUTLINE" in day mode and "BLACK" in night mode? 😛 The filenames in that image with the six examples shows you which is which.

@chrisbobbe
Copy link
Contributor

chrisbobbe commented Apr 4, 2020

Apple says:

Automatic translation of the button's title into the language specified by the device

Looks like the Apple-provided button translates based on the device-level language choice. (Do we have a default language selection that's based on the device language? Might be worth an issue.)

@gnprice
Copy link
Member Author

gnprice commented Apr 6, 2020

I think I prefer the button styles that look most like the other buttons.

Maybe, the "WHITE_OUTLINE" in day mode and "BLACK" in night mode?

Yep, those are the ones I had in mind 🙂

(Do we have a default language selection that's based on the device language? Might be worth an issue.)

Yeah, this is #2612 .

@chrisbobbe chrisbobbe removed the blocked on other work To come back to after another related PR, or some other task. label Apr 8, 2020
@chrisbobbe
Copy link
Contributor

(removed "blocked on other work" because Unimodules landed in #4016)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-iOS a-onboarding Everything you would do when first joining a realm. P1 high-priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants