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.

Sign up for GitHub

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

Jump to bottom

2786: Upgrade dependencies #2912

Open
wants to merge 26 commits into
base: 2786-upgrade-react-native-0.74
Choose a base branch
from
Open

2786: Upgrade dependencies #2912

wants to merge 26 commits into from

Conversation

steffenkleinle
Copy link
Member

@steffenkleinle steffenkleinle commented Sep 4, 2024 •
edited
Loading

Short description

Upgrade dependencies.

Proposed changes

  • Upgrade most dependencies
  • Increase memory limit for linting in CI

Blocked

  • @react-native-community/geolocation: Somehow crashes the app if using different location providers than google, see [TypeError: Cannot read property 'getCurrentPosition' of null] Agontuk/react-native-geolocation-service#433 (comment)
  • entities: htmlparser2 expects v4.5.0 and does not work otherwise
  • @dr.pogodin/react-native-static-server (starting v0.8): RN 0.72 and minimum SDK 28
  • @math.gl/web-mercator: Dependency not found?
  • react-navigation-header-buttons: Lots of changes necessary and using of unstable flags.
  • webdriverio: E2E tests fail without any apparent reason.
  • webpack-dev-server: Error: Cannot set headers after they are sent to the client stops the server in the CI
  • decamelize, flat: Only ESM modules available
  • eslint: Lots of breaking changes in v9.0.0. Might be done in a separate issue once bugs are fixed.
  • react-native and react: Blocked by react-native-static-server.

Side effects

Hopefully none.

Testing

⚠️ Test the whole app, both web and native!

Resolved issues

Fixes: #2786.

@steffenkleinle steffenkleinle changed the base branch from main to 2786-upgrade-react-native-0.74 September 4, 2024 10:06
@steffenkleinle steffenkleinle mentioned this pull request Sep 4, 2024
Open
@steffenkleinle steffenkleinle marked this pull request as ready for review September 4, 2024 12:58
LeandraH
LeandraH reviewed Sep 5, 2024
View reviewed changes
Copy link
Contributor

@LeandraH LeandraH left a comment

Choose a reason for hiding this comment

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

The code looks fine but I'm getting an error when trying to build it for iOS (Redefinition of module 'ReactCommon'), I'll try to fix that next week :)

ztefanie
ztefanie approved these changes Sep 10, 2024
View reviewed changes
Copy link
Member

@ztefanie ztefanie left a comment

Choose a reason for hiding this comment

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

Thanks for updating 🚀

quickly tested an Android and firefox, everything works as expected.

e2e-tests/web/test/pageobjects/page.ts
@@ -9,7 +9,11 @@ export default class Page {
* @param path path of the sub page (e.g. /path/to/page.html)
*/

open(path: string): Promise<string> {
return browser.url(`${path}`)
async open(path: string): Promise<string> {
Copy link
Member

Choose a reason for hiding this comment

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

🙃 nitpick: i personally dislike mixing async/await and Promises like this, but no need to change it.

Copy link
Member Author

Choose a reason for hiding this comment

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

How would you change this? I think I don't really mix async/await and promises, Promise is just the correct return type of a async function.

LeandraH
LeandraH approved these changes Sep 10, 2024
View reviewed changes
Copy link
Contributor

@LeandraH LeandraH left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for (still) slogging through that! Tested on Firefox and on real iOS, works nicely!

@steffenkleinle steffenkleinle mentioned this pull request Sep 16, 2024
Closed
f1sh1918
f1sh1918 reviewed Sep 18, 2024
View reviewed changes
Copy link
Contributor

@f1sh1918 f1sh1918 left a comment

Choose a reason for hiding this comment

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

Unfortunately i can't build the app on ios with Xcode 15.4

Steps so far:

  • yarn install
  • pod install (leads to differences in podfile.lock (ios folder), check my comment
  • "Clean product "in xcode
  • run build

Error
image

I was also checking this issue to fix it, but the resolved podfile.lock is still differing from your verion and the error remains
reactwg/react-native-releases#284
But couldn't get it work.
Even i run a build without pod install using your podfile.lock results in same error

native/ios/Podfile.lock Outdated
@@ -34,33 +57,72 @@ PODS:
- GCDWebServer/Core (= 3.5.4)
- GCDWebServer/Core (3.5.4)
- glog (0.3.5)
- GoogleAppMeasurement (10.24.0):
Copy link
Contributor

Choose a reason for hiding this comment

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

if i run pod install i don't get these resolved pods here. My podfile.lock differs a lot. Even i ran yarn install before. But i'm using newer cocoapods version and

@LeandraH
Copy link
Contributor

Urg, can you try deleting your node_modules and Pods and then reinstalling?

@steffenkleinle
Copy link
Member Author

One thing perhaps: After you installed the pods/did the installation for iOS @LeandraH, I reverted some upgrades and dependency resolutions. So differing pod file is probably okay, just commit them.

@steffenkleinle steffenkleinle mentioned this pull request Sep 18, 2024
Open
@LeandraH
Copy link
Contributor

It doesn't build for me anymore either, though that may partially be because I stupidly let my computer update Xcode this morning 🤦 I'm getting lots of "Could not build module XYZ" errors in React-RCTAppDelegate. Will try to figure that out tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@f1sh1918 f1sh1918 f1sh1918 left review comments

@ztefanie ztefanie ztefanie approved these changes

@LeandraH LeandraH LeandraH approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@steffenkleinle @LeandraH @ztefanie @f1sh1918