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: bump gradle wrapper version to 7.5.1 & not use app node_modules when running tasks from repo #2186

Merged
merged 2 commits into from
Jun 17, 2024

Conversation

kkafar
Copy link
Member

@kkafar kkafar commented Jun 17, 2024

Description

Basically tasks run from react-native-screens repo (such as linting & formating) would fail due to outdated gradle wrapper version (and the fact that we re using newer JDK now).

There was a second issue: when running tasks from react-native-screens project as gradle's rootProject and simultaneously having react-native repo side-by-side to react-native-screens
repo on disk - our current react native directory resolution algorithm would have found it and attempt to use to retrieve RN version, but:

  1. the gradle.properties file is not there,
  2. retrieved version could be out of sync with what's in project.

Changes

  • Bumped gradle wrapper version to 7.5.1 (we were still on major 6).
  • Adjusted the react native directory algorithm just a bit, so that when running in context of screens repo we skip one code path.

There was also possibility to "reorder the ifs" in resolveReactNativeDir function, however this could possible cause issues in user projects - we want first to look for app's node modules
and later for project node modules.

Test code and steps to reproduce

yarn lint-android, yarn format-android, etc. should now work just fine when you re on JDK 17+ & have react-native repo side by side to react-native-screens.

Checklist

Copy link
Member

@alduzy alduzy left a comment

Choose a reason for hiding this comment

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

Looks good 🚀

@kkafar kkafar merged commit cfc046f into main Jun 17, 2024
4 checks passed
@kkafar kkafar deleted the @kkafar/fix-gradle-once-and-for-all branch June 17, 2024 10:47
kkafar added a commit that referenced this pull request Jun 20, 2024
…when running tasks from repo (#2186)

## Description

Basically tasks run from react-native-screens repo (such as linting &
formating) would fail due to outdated gradle wrapper version (and the
fact that we re using newer JDK now).

There was a second issue: when running tasks from react-native-screens
project as gradle's `rootProject` and simultaneously having
`react-native` repo side-by-side to `react-native-screens`
repo on disk - our current react native directory resolution algorithm
would have found it and attempt to use to retrieve RN version, but:

1. the gradle.properties file is not there,
2. retrieved version could be out of sync with what's in project.


## Changes

* Bumped gradle wrapper version to 7.5.1 (we were still on major 6).
* Adjusted the react native directory algorithm just a bit, so that when
running in context of screens repo we skip one code path.

There was also possibility to "reorder the ifs" in
`resolveReactNativeDir` function, however this could possible cause
issues in user projects - we want first to look for app's node modules
and later for project node modules.


## Test code and steps to reproduce

`yarn lint-android`, `yarn format-android`, etc. should now work just
fine when you re on JDK 17+ & have `react-native` repo side by side to
`react-native-screens`.

## Checklist

- [ ] Included code example that can be used to test this change
- [ ] Updated TS types
- [ ] Updated documentation: <!-- For adding new props to native-stack
-->
- [ ]
https://github.com/software-mansion/react-native-screens/blob/main/guides/GUIDE_FOR_LIBRARY_AUTHORS.md
- [ ]
https://github.com/software-mansion/react-native-screens/blob/main/native-stack/README.md
- [ ]
https://github.com/software-mansion/react-native-screens/blob/main/src/types.tsx
- [ ]
https://github.com/software-mansion/react-native-screens/blob/main/src/native-stack/types.tsx
- [ ] Ensured that CI passes
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.

None yet

2 participants