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

[iOS] Update podspec to use install_modules_dependencies #2635

Merged

Conversation

cipolleschi
Copy link
Contributor

This change updates the RNGestureHandler.podspec to consume the install_modules_dependencies function provided by React Native to configure the pods dependencies.
Thanks to using this function, whenever we update the function in React Native, the library will automatically benefit from it.
This will make the library more future proof and more resilient to changes in React Native

This allow RNGestureHandler to work also with frameworks, as before the change, it failed to build when use_frameworks! was set in RN 0.72.x

Test plan

Tested locally:

Before changes

  1. Created a new app with npx react-native init RNFrameworks --version latest --skip-install
  2. cd RNFrameworks
  3. yarn add react-native-gesture-handler
  4. yarn install
  5. cd ios
  6. NO_FLIPPER=1 USE_FRAMEWORKS=static RCT_NEW_ARCH_ENABLED=1 bundle exec pod install
  7. open RNFrameworks
  8. build and run and observe it failing because it can't find files in react/utils and react/debug

After Changes

  1. Reinstall pods NO_FLIPPER=1 USE_FRAMEWORKS=static RCT_NEW_ARCH_ENABLED=1 bundle exec pod install
  2. open RNFrameworks
  3. build and run and observe it succeeding

@j-piasecki
Copy link
Member

Thanks for taking the time to make a PR, we've known of the method for a while though 😅. The problem with it is that it was added in RN 0.71 IIRC, and at the moment we support RN all the way down to 0.64 (with plans to drop versions below 0.67 soon, but the problem persists in that case).

I could add a check for React Native version, but finding it out is prone to breaking in more custom configurations (like monorepos), or simply use the method only when USE_FRAMEWORKS=static is set. Do you have a preference on this (or a better approach)?

@cipolleschi
Copy link
Contributor Author

Out of curiosity, why are you supporting down to 0.64, if we support only 70, 71, 72 and soon 73?
Wouldn't be better to align the support of the library to what we support in core?

Anyway, I think that for the time being, we can use the ruby's defined?() method to see whether that function exists and fallback to the prev implementation when it doesn't. I'll update the PR before EOD! :D

@cipolleschi cipolleschi force-pushed the cipolleschi/fix_podspec branch from 3e8234b to cd8f7d5 Compare October 16, 2023 13:25
@cipolleschi
Copy link
Contributor Author

updated! @j-piasecki

@j-piasecki
Copy link
Member

Out of curiosity, why are you supporting down to 0.64, if we support only 70, 71, 72 and soon 73?

We supported 0.64 for quite a while as a min version, and from the time we started till now, there wasn't a meaningful reason to drop it since many people still used it. Recently we wanted to do a little overhaul of the GH's code, which would mean using some of the newer APIs - when that lands we plan to support 0.67 and higher since, again, there are a lot of people still using those versions and supporting them comes without much cost.

@uffou
Copy link

uffou commented Oct 16, 2023

That's great! I will try to jumpstart 72 again after this is released.

@j-piasecki j-piasecki merged commit 2300af7 into software-mansion:main Oct 16, 2023
renovate bot referenced this pull request in valora-inc/wallet Oct 21, 2023
…4355)

[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
|
[react-native-gesture-handler](https://togithub.com/software-mansion/react-native-gesture-handler)
| [`^2.13.2` ->
`^2.13.3`](https://renovatebot.com/diffs/npm/react-native-gesture-handler/2.13.2/2.13.3)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/react-native-gesture-handler/2.13.3?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/react-native-gesture-handler/2.13.3?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/react-native-gesture-handler/2.13.2/2.13.3?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/react-native-gesture-handler/2.13.2/2.13.3?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>software-mansion/react-native-gesture-handler
(react-native-gesture-handler)</summary>

###
[`v2.13.3`](https://togithub.com/software-mansion/react-native-gesture-handler/releases/tag/2.13.3)

[Compare
Source](https://togithub.com/software-mansion/react-native-gesture-handler/compare/2.13.2...2.13.3)

#### 👍 Improvements

- \[iOS] Update podspec to use install_modules_dependencies by
[@&#8203;cipolleschi](https://togithub.com/cipolleschi) in
[https://github.com/software-mansion/react-native-gesture-handler/pull/2635](https://togithub.com/software-mansion/react-native-gesture-handler/pull/2635)

#### What's Changed

- Docs: Fix Brent Vatne's last name (Vetne -> Vatne) by
[@&#8203;peterpme](https://togithub.com/peterpme) in
[https://github.com/software-mansion/react-native-gesture-handler/pull/2634](https://togithub.com/software-mansion/react-native-gesture-handler/pull/2634)

#### New Contributors

- [@&#8203;cipolleschi](https://togithub.com/cipolleschi) made their
first contribution in
[https://github.com/software-mansion/react-native-gesture-handler/pull/2635](https://togithub.com/software-mansion/react-native-gesture-handler/pull/2635)

**Full Changelog**:
software-mansion/react-native-gesture-handler@2.13.2...2.13.3

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 5pm,every weekend" in timezone
America/Los_Angeles, Automerge - "after 5pm,every weekend" in timezone
America/Los_Angeles.

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/valora-inc/wallet).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xOS4yIiwidXBkYXRlZEluVmVyIjoiMzcuMTkuMiIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: valora-bot <[email protected]>
Co-authored-by: Jean Regisser <[email protected]>
github-merge-queue bot pushed a commit to software-mansion/react-native-reanimated that referenced this pull request Nov 21, 2023
## Summary

Based on the changes in our libraries mentioned in this PR
(software-mansion/react-native-gesture-handler#2635),
we need to make adjustments to our `.podspec` file. We should utilize
the `install_modules_dependencies` feature available in React Native
since version 0.72.

Backward compatibility:
<img width="126" alt="image"
src="https://github.com/software-mansion/react-native-reanimated/assets/36106620/ce5c1c67-8251-47d6-8d4b-98c959a4dfb8">

## Test plan

Check CI

---------

Co-authored-by: Tomek Zawadzki <[email protected]>
Latropos pushed a commit to software-mansion/react-native-reanimated that referenced this pull request Nov 24, 2023
## Summary

Based on the changes in our libraries mentioned in this PR
(software-mansion/react-native-gesture-handler#2635),
we need to make adjustments to our `.podspec` file. We should utilize
the `install_modules_dependencies` feature available in React Native
since version 0.72.

Backward compatibility:
<img width="126" alt="image"
src="https://github.com/software-mansion/react-native-reanimated/assets/36106620/ce5c1c67-8251-47d6-8d4b-98c959a4dfb8">

## Test plan

Check CI

---------

Co-authored-by: Tomek Zawadzki <[email protected]>
Latropos pushed a commit to software-mansion/react-native-reanimated that referenced this pull request Dec 12, 2023
## Summary

Based on the changes in our libraries mentioned in this PR
(software-mansion/react-native-gesture-handler#2635),
we need to make adjustments to our `.podspec` file. We should utilize
the `install_modules_dependencies` feature available in React Native
since version 0.72.

Backward compatibility:
<img width="126" alt="image"
src="https://github.com/software-mansion/react-native-reanimated/assets/36106620/ce5c1c67-8251-47d6-8d4b-98c959a4dfb8">

## Test plan

Check CI

---------

Co-authored-by: Tomek Zawadzki <[email protected]>
Latropos pushed a commit to software-mansion/react-native-reanimated that referenced this pull request Dec 18, 2023
Based on the changes in our libraries mentioned in this PR
(software-mansion/react-native-gesture-handler#2635),
we need to make adjustments to our `.podspec` file. We should utilize
the `install_modules_dependencies` feature available in React Native
since version 0.72.

Backward compatibility:
<img width="126" alt="image"
src="https://github.com/software-mansion/react-native-reanimated/assets/36106620/ce5c1c67-8251-47d6-8d4b-98c959a4dfb8">

Check CI

---------

Co-authored-by: Tomek Zawadzki <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants