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

refactor: FrescoBasedTextInlineImageSpan #50532

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

gouravkhunger
Copy link

@gouravkhunger gouravkhunger commented Apr 7, 2025

Summary:

I've migrated FrescoBasedTextInlineImageSpan.java to kotlin. Reference #50513.

Changelog:

[ANDROID] [CHANGED] - Refactor class FrescoBasedTextInlineImageSpan from Java to Kotlin.

Test Plan:

Tested the RN tester app with yarn android on both new and old architecture.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 7, 2025
@gouravkhunger
Copy link
Author

Hi @mateoguzmana, I'm mostly done with the migration but facing some error with the AbstractDraweeControllerBuilder class expecting 4 type arguments. I've tried wildcard types

private val draweeControllerBuilder: AbstractDraweeControllerBuilder<*, *, ImageRequest, *>,

But this line is erroring out when trying to test with yarn android

BUILD FAILED in 4s
error Failed to install the app. Command failed with exit code 1: ./gradlew installHermesDebug -PreactNativeDevServerPort=8081
e: file:///Users/gourav/temp/react-native/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/frescosupport/FrescoBasedReactTextInlineImageSpan.kt:133:53 Argument type mismatch: actual type is 'com.facebook.imagepipeline.request.ImageRequest', but 'CapturedType(*)?' was expected. FAILURE: Build failed with an exception. * What went wrong:
Execution failed for task ':packages:react-native:ReactAndroid:compileDebugKotlin'.
> A failure occurred while executing org.jetbrains.kotlin.compilerRunner.GradleCompilerRunnerWithWorkers$GradleKotlinCompilerWorkAction > Compilation error. See log for more details * Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.
> Get more help at https://help.gradle.org. BUILD FAILED in 4s.
info Run CLI with --verbose flag for more details.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

I'm not sure about the usage of that class' types, as the java variant didn't specifically define those. Could you please help a little? Thank you so much!

private val tintColor: Int,
uri: Uri?,
private val headers: ReadableMap?,
private val draweeControllerBuilder: AbstractDraweeControllerBuilder<*, *, ImageRequest, *>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Try:

Suggested change
private val draweeControllerBuilder: AbstractDraweeControllerBuilder<*, *, ImageRequest, *>,
private val draweeControllerBuilder: AbstractDraweeControllerBuilder<*, ImageRequest, *, *>,

Was a bit tricky to understand as well, but this worked

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for this! It worked. My bad I was passing ImageRequest as the third param while the class signature expects it to be on the 2nd:

https://github.com/facebook/fresco/blob/b79964050f2bd82c8e6e9bc5a96dcfe61f95ae67/drawee/src/main/java/com/facebook/drawee/controller/AbstractDraweeControllerBuilder.java#L36

@gouravkhunger gouravkhunger force-pushed the gourav/migrate-to-kotlin branch from 13bf39a to e43f277 Compare April 8, 2025 07:21
@gouravkhunger gouravkhunger marked this pull request as ready for review April 8, 2025 07:31
@facebook-github-bot facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Apr 8, 2025
@gouravkhunger gouravkhunger force-pushed the gourav/migrate-to-kotlin branch from 45e9048 to 86d7e7c Compare April 8, 2025 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants