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

Multiple version checking #4914

Merged
merged 35 commits into from
Sep 27, 2023
Merged

Multiple version checking #4914

merged 35 commits into from
Sep 27, 2023

Conversation

tjzel
Copy link
Contributor

@tjzel tjzel commented Aug 10, 2023

Summary

This PR is a pinnacle of Reanimated's version detection grand scheme.

Since there are numerous errors stemming from having parts of Reanimated used from different version we decided to enforce runtime checks for those parts - to make the troubleshooting process easier.

Adding:

  • C++ checks JavaScript version
  • C++ checks Java version
  • Java checks C++ version

Requires #4915

Test plan

  1. C++ checks JavaScript version:

Trigger failed resolution:

// src/reanimated2/NativeReanimated/NativeReanimated.ts
-  global._REANIMATED_VERSION_JS = jsVersion;

Trigger wrong version:

// src/reanimated2/NativeReanimated/NativeReanimated.ts
- global._REANIMATED_VERSION_JS = jsVersion;
+ global._REANIMATED_VERSION_JS = 'wrong-version';
  1. C++ checks Java version:

Trigger failed resolution:

// android/src/main/java/com/swmansion/reanimated/nativeProxy/NativeProxyCommon.java
  public String getReanimatedJavaVersion() {
-   return BuildConfig.REANIMATED_VERSION_JAVA;
+   throw new RuntimeException("forced fail");
  }

Trigger wrong version:

// android/src/main/java/com/swmansion/reanimated/nativeProxy/NativeProxyCommon.java
  public String getReanimatedJavaVersion() {
-   return BuildConfig.REANIMATED_VERSION_JAVA;
+   return new String("wrong-version");
  }
  1. Java checks C++ version:

Trigger failed resolution:

// android/src/main/java/com/swmansion/reanimated/nativeProxy/NativeProxyCommon.java
  protected void setCppVersion(String version) {
-   cppVersion = version;
+   // NOOP
  }

Trigger wrong version:

// android/src/main/java/com/swmansion/reanimated/nativeProxy/NativeProxyCommon.java
  protected void setCppVersion(String version) {
-   cppVersion = version;
+   cppVersion = new String("wrong-version");
  }

@tjzel tjzel changed the base branch from main to @tjzel/troubleshooting August 21, 2023 10:36
Common/cpp/SharedItems/Shareables.cpp Outdated Show resolved Hide resolved
Common/cpp/Tools/RuntimeDecorator.cpp Outdated Show resolved Hide resolved
Common/cpp/Tools/RuntimeDecorator.cpp Outdated Show resolved Hide resolved
Common/cpp/Tools/RuntimeDecorator.cpp Outdated Show resolved Hide resolved
Common/cpp/Tools/RuntimeDecorator.cpp Outdated Show resolved Hide resolved
android/src/main/cpp/NativeProxy.cpp Outdated Show resolved Hide resolved
android/src/main/cpp/NativeProxy.cpp Outdated Show resolved Hide resolved
src/reanimated2/NativeReanimated/NativeReanimated.ts Outdated Show resolved Hide resolved
src/reanimated2/platform-specific/checkCppVersion.ts Outdated Show resolved Hide resolved
Base automatically changed from @tjzel/troubleshooting to main August 22, 2023 18:12
github-merge-queue bot pushed a commit that referenced this pull request Aug 22, 2023
<!-- Thanks for submitting a pull request! We appreciate you spending
the time to work on these changes. Please follow the template so that
the reviewers can easily understand what the code changes affect. -->

## Summary

Since version checker PR #4914 will add more error messages to
Reanimated and our Troubleshooting section in docs was almost
non-existent I decided to add a new one.

To make it consistent I made all errors thrown by Reanimated start with
`[Reanimated]` and prettified them a bit.

With this I made this tiny script `validate-urls.js` that scans our
source code to get valid (not so exotic) URLs and checks if they are
available.

## Test plan

CI
@tjzel tjzel marked this pull request as ready for review August 25, 2023 10:31
@piaskowyk piaskowyk self-requested a review August 28, 2023 08:12
Common/cpp/Tools/ReanimatedVersion.cpp Outdated Show resolved Hide resolved
Common/cpp/Tools/ReanimatedVersion.h Outdated Show resolved Hide resolved
RNReanimated.podspec Show resolved Hide resolved
android/build.gradle Show resolved Hide resolved
Common/cpp/Tools/ReanimatedVersion.cpp Outdated Show resolved Hide resolved
docs/docs/guides/troubleshooting.mdx Outdated Show resolved Hide resolved
docs/docs/guides/troubleshooting.mdx Outdated Show resolved Hide resolved
src/reanimated2/NativeReanimated/NativeReanimated.ts Outdated Show resolved Hide resolved
src/reanimated2/NativeReanimated/NativeReanimated.ts Outdated Show resolved Hide resolved
@tjzel tjzel requested a review from tomekzaw September 19, 2023 09:53
Copy link
Member

@piaskowyk piaskowyk left a comment

Choose a reason for hiding this comment

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

inblanco 🚀

src/reanimated2/platform-specific/checkCppVersion.ts Outdated Show resolved Hide resolved
Common/cpp/Tools/ReanimatedVersion.h Show resolved Hide resolved
scripts/validate-urls.js Outdated Show resolved Hide resolved
@piaskowyk piaskowyk merged commit df30b68 into main Sep 27, 2023
20 checks passed
@piaskowyk piaskowyk deleted the @tjzel/version-checker branch September 27, 2023 08:58
@Latropos
Copy link
Contributor

@tjzel Since this is merged, can we close #5029 now?

@tjzel
Copy link
Contributor Author

tjzel commented Oct 18, 2023

@Latropos yes!

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

4 participants