Update RCTLinkingManager.h to explicitly state the 'nullability' of parameters

[noxee]

Fixes #20797

As mentioned in #20797 when running react-native run-ios Xcode 9.2 will complain about the nullability of pointers in RCTLinkingManager.h`.

Test Plan:

My testing method probably isn't the best as I'm not sure what the best way to go about testing native code in isolation but I verified these changer worked by doing the following:

1. Run and see build fail

react-native init TestProject --version="0.57.0-rc.0"
cd TestProject
react-native run-ios

2. Update node_modules/react-native/Libraries/LinkingIOS/RCTLinkingManager.h with the changes in this pull request.
3. Re-run react-native run-ios and verify build succeeds.

Release Notes:

[IOS] [BUGFIX] [RCTLinkingManager] - Update method signatures in RCTLinkingManager.h to prevent build errors with Xcode 9.2.

[facebook-github-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[DimitryDushkin]

That's the real problem. Would be nice to land it in 57

[kelset]

Hey everyone, we didn't see this PR - seems like a fairly small patch so we'll try to have it review & merged for 0.57.1

[pkyeck]

@kelset thanks for the reply.

But I was sincerely wondering how a "bug" like this can make it to production?! As noxee wrote, with 0.57 you can't run a newly setup project on iOS - there should be some kind of e2e- or integration- or whatever-tests covering the basic usage (not a super random edgecase thing) so newcomers won't run into walls after 2min with react-native.

I don't want to throw anyone under the bus - don't get me wrong - just saying that things like these are costing a lot of people a lot of time and nerves and are the main reason why other people see RN as not mature enough (I guess).

Other than that, thank you for the work you're all putting into RN! 👍

[noxee]

@pkyeck I'm mostly to blame for this not being seen. I submitted the PR and then didn't communicate with the React Native team when they were stabilizing 0.57 to see if it could make it into the release.

There's a lot of discussions and work going into resolving some of the pain points in releases so I think in the future this kind of problem will be mitigated.

[pkyeck]

There's a lot of discussions and work going into resolving some of the pain points in releases so I think in the future this kind of problem will be mitigated.

Great to hear this!

[wuhuangpeng]

this promble occur on xcode 9.2, xcode version >= 9.3 has not this promble

[radko93]

@kelset any idea who could review that? It's blocking everyone on an older Mac (as you cannot use Xcode > 9.2 without High Sierra).

[grabbou]

Looks fairly right. Seen similar update in one of the other components back in the day.

[kelset]

(quick update: tested it locally, everything seems good - just waiting for @radko93 to double check and then I'll proceed to merge it)

[ikesyo]

Looks like this parameter should be nonnull.

https://developer.apple.com/documentation/uikit/uiapplicationdelegate/1623112-application

options: [UIApplication.OpenURLOptionsKey : Any] = [:]

By default, the value of this parameter is an empty dictionary.

[radko93]

@kelset @grabbou I can confirm that the build is successful with this PR and on Xcode 9.2 (and I could reproduce the issue before). Also works with nonnull as @ikesyo suggested.

[kelset]

Thanks @radko93 - I'll wait for @noxee to update the PR with the fix required by @ikesyo then I'll proceed to merge 👍

[noxee]

@kelset Sorry for the delay, I've made the fix that @ikesyo pointed out but it looks like analyze_pr CI is currently failing due to some bad credentials.

[kelset]

Thanks for the update!
I don't think it will matter, given that tests were all green earlier && @radko93 test both scenarios 👍

@facebook-github-bot shipit

[facebook-github-bot]

kelset is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[react-native-bot]

Warren Knox merged commit 2271d1f into facebook:master.

---

Once this commit is added to a release, you will see the corresponding version tag below the description at 2271d1f. If the commit has a single master tag, it is not yet part of a release.