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

Mobile: Upgrade to React Native 0.74.1 #10401

Open
wants to merge 22 commits into
base: dev
Choose a base branch
from

Conversation

personalizedrefrigerator
Copy link
Collaborator

@personalizedrefrigerator personalizedrefrigerator commented May 5, 2024

Summary

This pull request:

  1. Applies the diff provided by the React Native upgrade helper.
  2. Migrates from @react-native-community/clipboard to @react-native-clipboard/clipboard. @react-native-community/clipboard doesn't build with RN 0.73 and seems to have been renamed to @react-native-clipboard/clipboard.
  3. Upgrades our fork of react-native-alarm-notification for compatibility with newer React Native versions.

Note

React Native 0.74 removes default support for Flipper.

To-do

  • Upgrade our patches written for react-native v0.71 to work with v0.74.
    • Check whether they're still necessary?
  • Write a testing plan and verify that features still work on iOS and Android. (See below)

Testing plan

Note

Manual testing in-progress. New issues may include:

  • iOS: Pictures taken with the camera are inserted multiple times.
    • This will need to be verified with a build of Joplin from the dev branch.
  • iOS
    • Sync
      1. Create a new profile
      2. Set up sync with a local copy of Joplin Server
      3. Create a notebook
      4. Enable encryption if not already enabled
      5. Add a note, attach a drawing
      6. Sync
      7. Verify that the note and drawing are visible on another client after sync.
      8. Edit the drawing on the other client and sync
      9. Sync the original mobile client and verify that the changes to the drawing are visible.
    • Plugins
      1. Enable plugin support and install the "note tabs" and "quick links" plugins
      2. Create a new note
      3. Link to the note created while testing sync above with the "quick links" plugin
      4. Open the note viewer and click on the link
      5. Open the "note tabs" panel
      6. Close the "note tabs" panel
    • Camera
      1. Open the note editor.
      2. Click on the paperclip icon in the toolbar.
      3. Click "take photo".
      4. Take a picture.
        • Possible issue: This attaches two copies of the photo to the note (with the same id). This doesn't happen if I instead click "attach photo" to attach an existing photo.
      5. Close the note editor.
      6. Verify that a picture was attached to the note.
      7. Open the note actions menu.
      8. Click "Attach"
      9. Click "Attach photo"
      10. Attach a photo.
      11. Repeat steps 7-10 for "Attach file"
    • Export and import (& the share sheet)
      1. Open "Configuration" > "Import and Export"
      2. Click "Export all notes as JEX"
      3. Save the exported JEX file to the filesystem.
      4. Import the JEX file using "Import from JEX"
    • Joplin Server share (tests react-native-rsa).
      1. From another Joplin Server account (with encryption enabled), share a notebook with the account the mobile device is using.
      2. Accept the share
      3. Sync
      4. Add a note to the shared notebook
      5. Sync
      6. Verify that the new note is visible on a different client.
  • Android
    • Sync
      1. Create a new profile
      2. Set up sync with a local copy of Joplin Server
      3. Enable encryption if not already enabled
      4. Add a note, attach a drawing
      5. Sync
      6. Verify that the note and drawing are visible on another client after sync.
    • Plugins
      1. Enable plugin support and install the "note tabs" and "quick links" plugins
      2. Create a new note
      3. Link to the note created while testing sync above with the "quick links" plugin
      4. Open the "note tabs" panel
      5. Close the "note tabs" panel
    • Camera
      1. Open the note editor.
      2. Click on the paperclip icon in the toolbar.
      3. Click "take photo".
      4. Take a picture.
      5. Close the note editor.
      6. Verify that a picture was attached to the note.
      7. Open the note actions menu.
      8. Click "Attach"
      9. Click "Attach file"
      10. Attach a file
    • Export and import (& the share sheet)
      1. Open "Configuration" > "Import and Export"
      2. Click "Export all notes as JEX"
      3. Save the exported JEX file to the filesystem.
        • On Android, only sharing is implemented. To test this, I shared to Termux, which saved the exported JEX to the filesystem.
      4. Import the JEX file using "Import from JEX"
    • Joplin Server share (tests react-native-rsa)
      1. From another Joplin Server account (with encryption enabled), share a notebook with the account the mobile device is using.
      2. Sync and accept the share
      3. Sync
      4. Add a note to the shared notebook
      5. Sync
    • File system sync
      1. Change the sync target to "file system"
      2. Sync
      3. Verify that sync completes successfully
    • Voice typing
      1. Open the note actions menu and click "Voice typing..."
      2. Wait for the language model to download.
      3. Grant permission and slowly say: "This is a test of voice typing".

Note: The encrypted share tests were re-done after a fix was merged separately.

@laurent22
Copy link
Owner

There's an error on CI:

FAILURE: Build failed with an exception.

* Where:
Build file '/home/runner/work/joplin/joplin/packages/app-mobile/android/app/build.gradle' line: 1

* What went wrong:
A problem occurred evaluating project ':app'.
> Failed to apply plugin 'com.android.internal.application'.
   > Android Gradle plugin requires Java 17 to run. You are currently using Java 11.
      Your current JDK is located in /usr/lib/jvm/temurin-11-jdk-amd64
      You can try some of the following options:
       - changing the IDE settings.
       - changing the JAVA_HOME environment variable.
       - changing `org.gradle.java.home` in `gradle.properties`.

Maybe we can use this to setup Java on the action? https://github.com/actions/setup-java

@laurent22
Copy link
Owner

By the way is this pull request related to the Dropbox issue?

@personalizedrefrigerator
Copy link
Collaborator Author

By the way is this pull request related to the Dropbox issue?

This pull request should allow publishing new iOS releases. Aside from that, this is unrelated to the sync issue.

@personalizedrefrigerator personalizedrefrigerator changed the title Mobile: Upgrade to React Native 0.73.8 Mobile: Upgrade to React Native 0.74.1 May 10, 2024
@personalizedrefrigerator personalizedrefrigerator marked this pull request as ready for review May 10, 2024 23:18
Comment on lines -87 to -90
def reactNativeArchitectures() {
def value = project.getProperties().get("reactNativeArchitectures")
return value ? value.split(",") : ["armeabi-v7a", "x86", "x86_64", "arm64-v8a"]
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This code was removed upstream. See android/app/build.gradle in the upgrade diff.

Comment on lines -78 to -80
// We need the intl variant to support natural sorting of notes.
// https://github.com/laurent22/joplin/pull/4272
def jscFlavor = 'org.webkit:android-jsc-intl:+'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We no longer use JSC. Hermes should support Intl natively.

packages/app-mobile/ios/PrivacyInfo.xcprivacy Outdated Show resolved Hide resolved
Comment on lines -107 to -127
// some Gradle build hooks ref:
// https://www.oreilly.com/library/view/gradle-beyond-the/9781449373801/ch03.html
task androidJavadoc(type: Javadoc) {
source = android.sourceSets.main.java.srcDirs
classpath += files(android.bootClasspath)
// Must be removed due to error "Configuration with name 'compile' not found."
// classpath += files(project.getConfigurations().getByName('compile').asList())
include '**/*.java'
}

task androidJavadocJar(type: Jar, dependsOn: androidJavadoc) {
classifier = 'javadoc'
from androidJavadoc.destinationDir
}

task androidSourcesJar(type: Jar) {
classifier = 'sources'
from android.sourceSets.main.java.srcDirs
include '**/*.java'
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This applies an upstream change from the original react-native-alarm-notification. See 0491cae.

<key>NSPrivacyAccessedAPIType</key>
<string>NSPrivacyAccessedAPICategoryFileTimestamp</string>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The NSPrivacyAccessedAPICategoryFileTimestamp API was present twice in this file. XCode automatically removed the second occurrence.

Comment on lines +42 to +46
<!-- Do not change NSAllowsArbitraryLoads to true, or you will risk app rejection! -->
<key>NSAllowsArbitraryLoads</key>
<false/>
<key>NSAllowsLocalNetworking</key>
<true/>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This change is from the upgrade diff. However, it may address #10420 (untested).

Copy link
Contributor

Choose a reason for hiding this comment

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

IOS17.2.1 + Joplin 12.14.8 code + this modify(NSAllowsLocalNetworking), not fix #10420 .
I found a alternatives way: #10437

Copy link
Owner

Choose a reason for hiding this comment

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

Not sure there's anything we can do here since we can only whitelist specific hosts? Or maybe we could whitelist a generic host like sync.server and document this somewhere? Then the user can do their own setup and use this hostname to get it working

Comment on lines -718 to +724
IPHONEOS_DEPLOYMENT_TARGET = 11.0;
IPHONEOS_DEPLOYMENT_TARGET = 13.4;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This upgrade increases the minimum supported iOS version to 13.4.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Locally,

$ sha256sum gradle-wrapper.jar 
0336f591bc0ec9aa0c9988929b93ecc916b3c1d52aed202c7381db144aa0ef15  gradle-wrapper.jar

This matches the checksum for "Wrapper JAR Checksum" in v8.4 on https://gradle.org/release-checksums/ .

Comment on lines -5 to +6
buildToolsVersion = "33.0.0"
minSdkVersion = 21
buildToolsVersion = "34.0.0"
minSdkVersion = 23
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This increases the minimum supported Android version. minSdkVersion = 23 corresponds to Android 6.

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

3 participants