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: override fun onNewIntent not nullable param #564

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

lucashe1997
Copy link

@lucashe1997 lucashe1997 commented Dec 27, 2024

Summary

While trying to implement this package, I encountered a problem when using Expo 52 with a development build, specifically with the Kotlin language during the Android build:

Android gradle plugin: 8.6.0
Gradle: 8.10.2

> Task :app:compileDebugKotlin FAILED
e: file:///.../android/app/src/main/java/.../MainActivity.kt:69:3 'onNewIntent' overrides nothing
e: file:///.../android/app/src/main/java/.../MainActivity.kt:70:23 Type mismatch: inferred type is Intent? but Intent was expected

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':app:compileDebugKotlin'.
> A failure occurred while executing org.jetbrains.kotlin.compilerRunner.GradleCompilerRunnerWithWorkers$GradleKotlinCompilerWorkAction
   > Compilation error. See log for more details

After investigating the issue, I found that the problem originates from the following piece of code:

  override fun onNewIntent(intent: Intent?) {
    super.onNewIntent(intent)
    intent?.let { AdyenCheckout.handleIntent(it) }
  }

The Android documentation for onNewIntent specifies that the intent parameter is non-nullable. As a result, the null check (intent?.let) is unnecessary. This behavior is consistent with the Java implementation provided in the same plugin:

  @Override
  public void onNewIntent(Intent intent) {
    super.onNewIntent(intent);
    AdyenCheckout.handleIntent(intent);
  }

@Zorig
Copy link

Zorig commented Jan 3, 2025

faced the same issue @jreij

@descorp
Copy link
Contributor

descorp commented Jan 3, 2025

Hey @lucashe1997

Thank you for making PR! 💚

This breaking change introduced by Google on Android API level 35.

Before we merge this pull request, we ideally need to ensure support for older API levels as well. I am currently exploring ways to achieve this.

Alternatively, we are considering changing the implementation to use a Consumer<Intent> instead of overriding onNewIntent, but this will require a major version release.

@robyhz
Copy link

robyhz commented Jan 8, 2025

any update ?

@vanGalilea
Copy link

@descorp can we expedite a fix release for this. please?

@descorp
Copy link
Contributor

descorp commented Jan 16, 2025

Hey @vanGalilea

Merging this pull request will cause integration issues for anyone who is not using API level 35 yet. Unfortunately, we will need to ask you to implement a manual fix until we can find a comprehensive solution or until API level 35 becomes widely adopted.

Thanks for checking!

@lucashe1997
Copy link
Author

Hi @vanGalilea as @descorp mentioned this PR solves the issue for the API level 35 and later, but it'll be a problem with older APIs.

If you are using expo, you can apply get the patch file https://patch-diff.githubusercontent.com/raw/Adyen/adyen-react-native/pull/564.patch and apply it using patch-package for example. It not you can change the code manually.

@descorp if this PR is causing too much confusion we can close it, and I can create just a issue, with the patch. Just let me know!

@descorp
Copy link
Contributor

descorp commented Jan 16, 2025

Thanks @lucashe1997 💚

PR is perfect! :D
We will address this issue eventually. It is not a priority right now since it is not a blocker and a workaround is available.

@vanGalilea
Copy link

Something didn't work with the suggested patch. I suspect this early exit in withAdyenAndroid.ts

I solved this by applying the following custom plugin, see gist.

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.

5 participants