Migrate Arguments to Kotlin#52457
Conversation
facebook-github-bot
added
the
CLA Signed
mateoguzmana
force-pushed
the
refactor/Arguments
branch
from
4a91535 to
63a7260
Compare
mateoguzmana
force-pushed
the
refactor/Arguments
branch
from
63a7260 to
71a9a9c
Compare
facebook-github-bot
added
the
Shared with Meta
cortinico
left a comment
cortinico
left a comment
There was a problem hiding this comment.
Just one nit for now, but I'll do a deeper review 👍
| public fun createMap(): WritableMap = WritableNativeMap() | ||
|
|
||
| @JvmStatic | ||
| public fun fromJavaArgs(args: Array<Any?>): WritableNativeArray { |
There was a problem hiding this comment.
This should probably be a:
| public fun fromJavaArgs(args: Array<Any?>): WritableNativeArray { | |
| public fun fromJavaArgs(vararg args: Any?): WritableNativeArray { |
There was a problem hiding this comment.
Been trying to use vararg here, but not with much luck. Once I do this change, I get a crash for a missing Object handler: java.lang.RuntimeException: Cannot convert argument of type class [Ljava.lang.Object; and amending the consumers of this class to try to make it happy doesn't seem to work either, I think it changes the logic
There was a problem hiding this comment.
@mateoguzmana when is the crash happening? Upon startup?
There was a problem hiding this comment.
Ok I was able to circumvent this. If all is good, I should be able to merge it with the vararg syntax
There was a problem hiding this comment.
Yes, happens upon startup indeed.
Ok I was able to circumvent this. If all is good, I should be able to merge it with the vararg syntax
Ah awesome!
Summary: This came out of #52457 as I had to fix some tests and realised there is a lot of setup that could be shadow and other things that are not needed at all. ## Changelog: [INTERNAL] - Clean up unnecessary test setup for Android tests Pull Request resolved: #52471 Test Plan: ```sh yarn test-android ``` Reviewed By: cortinico Differential Revision: D77926887 Pulled By: javache fbshipit-source-id: ff493d87633fcb4c4194b50cd374ad2e8acda974
mateoguzmana
force-pushed
the
refactor/Arguments
branch
from
ce03e44 to
d8fdef5
Compare
|
@cortinico has imported this pull request. If you are a Meta employee, you can view this in D78353290. |
|
@cortinico merged this pull request in 2534aea. |
|
This pull request was successfully merged by @mateoguzmana in 2534aea When will my fix make it into a release? | How to file a pick request? |
4 participants
Summary:
Migrate com.facebook.react.bridge.Arguments to Kotlin.
Changelog:
[Android][Changed] - Migrated com.facebook.react.bridge.Arguments to Kotlin.
Test Plan: