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 13 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!

@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
Copy link
Collaborator

@mateoguzmana mateoguzmana left a comment

Choose a reason for hiding this comment

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

Thanks for migrating this file! One small thing that would be nice to check about the null safety

@mateoguzmana mateoguzmana requested a review from mdvacca April 9, 2025 19:09
Co-authored-by: Mateo Guzmán <[email protected]>
@gouravkhunger
Copy link
Author

Hi @mateoguzmana @mdvacca gentle nudge. Is there anything else you need from my side? Thank you so much!

@mateoguzmana
Copy link
Collaborator

@gouravkhunger, nothing else is needed from your side for now. Once the second reviewer has availability, this will be imported and merged if there are no further things to fix

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