Skip to content

Breaking changes in 3.0 #1600

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

Merged
merged 9 commits into from
Jan 10, 2024
Merged

Breaking changes in 3.0 #1600

merged 9 commits into from
Jan 10, 2024

Conversation

tido64
Copy link
Member

@tido64 tido64 commented Sep 21, 2023

Description

This PR collects all breaking changes for the upcoming 3.0 cut.

Important
To preserve history, this PR must be rebased before merging.
image

Resolves #1153.

Blocked by:

Platforms affected

  • Android
  • iOS
  • macOS
  • Windows

Test plan

npm run set-react-version 0.66
yarn
# Switch to Node 16
cd example
npm run android
npm run ios

# Switch to Node 18+

# This should throw an exception when it tries to build with new architecture enabled
node scripts/test-matrix.mjs 0.70

# Enabling new architecture should… enable new architecture
node scripts/test-matrix.mjs 0.71
node scripts/test-matrix.mjs 0.73

@github-actions github-actions bot added platform: Android This affects Android platform: iOS This affects iOS platform: macOS This affects macOS platform: Windows This affects Windows labels Sep 21, 2023
@tido64 tido64 mentioned this pull request Sep 21, 2023
16 tasks
@tido64 tido64 force-pushed the tido/3.0 branch 3 times, most recently from 8739748 to 32441df Compare September 25, 2023 20:57
@tido64 tido64 force-pushed the tido/3.0 branch 3 times, most recently from 1c5dfe6 to ee1b1a2 Compare September 29, 2023 13:47
@tido64 tido64 force-pushed the tido/3.0 branch 8 times, most recently from 884fcb6 to 09f11e0 Compare October 17, 2023 09:43
@tido64 tido64 force-pushed the tido/3.0 branch 7 times, most recently from 60c163f to fe4ed70 Compare October 30, 2023 14:33
@tido64 tido64 force-pushed the tido/3.0 branch 2 times, most recently from cc1cd6e to c0c1bf7 Compare November 30, 2023 17:39
@kelset kelset self-assigned this Dec 5, 2023
@kelset
Copy link
Contributor

kelset commented Dec 18, 2023

update on my previous comment: it's all done, everything is working.

BUT, to make it all work seamlessly, aside from Tommy's fix for the C++20 stuff, I had to do two minor bandaids local commits:

  • one to make the android patch only apply if we're trying the 0.73 matrix, since it changes Gradle to 8.x. I'm not sure if we should keep it like this or have some more substantial change somewhere else so that when using 0.73 it will also change gradle side files. (commit)
  • the second one is better handling of the fact that we can't assume that the latest version releases of a package is a non-nightly in non-nightly scenarios. More details in the previous comment. Might be worth doing a better implementation so that what I wrote as an inline comment is handled "more properly". (commit)

I think that ideally what we should do next is to go in main branch, address those things proper, then after merging there, revert my local bandaids and instead cherry pick the proper solution commits from main, and then we should be good to go with this.

@kelset
Copy link
Contributor

kelset commented Jan 2, 2024

update: my second point has been addressed by #1753

and the first point by #1730

@tido64 tido64 marked this pull request as ready for review January 4, 2024 16:03
@tido64 tido64 requested a review from kelset January 4, 2024 16:04
tido64 added 8 commits January 8, 2024 15:43
BREAKING CHANGE: Dropped support for Node 14
BREAKING CHANGE: Dropped support for `react-native` 0.64, 0.65
BREAKING CHANGE: Dropped support for Flipper

For more context, see
react-native-community/discussions-and-proposals#641.
BREAKING CHANGE: `react-native` 0.71 is now required for New Architecture
BREAKING CHANGE: Bumping C++ language standard to C++20 breaks React
Native 0.65 and below.
@kelset
Copy link
Contributor

kelset commented Jan 8, 2024

one more round of testing before going ahead and merging.

Testing 0.66 ✅

It worked:
Screenshot 2024-01-09 at 16 48 11

A few finicky things:

  • pod install had to be run manually and was failing because of Folly, it needed a pod update RCT-Folly --no-repo-update
  • the npm run ios command was failing 'cause it wanted to run an iPhone 13, and completely ignored when I tried to pass the option --simulator="iPhone 15". Nothing big, I ran the iOS app from Xcode directly and it worked.
  • had to pass the ENV option to make Metro run: YARN_IGNORE_NODE=1 yarn start --reset-cache
Testing 0.70 ✅

Screenshot of it working:
Screenshot 2024-01-09 at 17 55 28

Notes:

  • just like last time, had to manually patch locally the WDIO config to bump the iOS version to 17.2
  • in order to make it work, I had to add to L133's options retryDelay: 500, (ref) so that the rm command would work successfully (without it, it would fail with race conditions)
  • it, correctly, errors out when trying to do new arch:
Screenshot 2024-01-09 at 17 55 38
Testing 0.71 ✅

Screenshot of it working:
Screenshot 2024-01-10 at 15 03 54

Notes:

  • went flawlessly, probably because of the local commits I did for 0.70 (the retryDelay and WDIO changes)
Testing 0.73 ✅

Screenshot of it working:
Screenshot 2024-01-10 at 15 29 28

Notes:

  • went flawlessly, probably because of the local commits I did for 0.70 (the retryDelay and WDIO changes)

@tido64 tido64 force-pushed the tido/3.0 branch 3 times, most recently from b2721bd to d35afd4 Compare January 8, 2024 16:27
Copy link
Contributor

@kelset kelset left a comment

Choose a reason for hiding this comment

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

I did one more full round of testing, overall I think we are ready to go!

The only minor thing is that tweak to the options for the retryDelay but I'll send that over as a separate small PR.

Let's 🚢 this bad boy!

@tido64
Copy link
Member Author

tido64 commented Jan 10, 2024

The only minor thing is that tweak to the options for the retryDelay but I'll send that over as a separate small PR.

I also did a full round. It's strange that I never hit this retryDelay issue on 0.70. Regardless, thanks for the thorough testing ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: Android This affects Android platform: iOS This affects iOS platform: macOS This affects macOS platform: Windows This affects Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Breaking changes in 3.0
2 participants