[expo-modules-core] Rework compose integration.#40653
Conversation
|
Subscribed to pull request
Generated by CodeMention |
|
The Pull Request introduced fingerprint changes against the base commit: ccdf9c5 Fingerprint diff[
{
"op": "changed",
"beforeSource": {
"type": "dir",
"filePath": "../../packages/expo-modules-core",
"reasons": [
"expoAutolinkingIos",
"expoAutolinkingAndroid",
"expoAutolinkingIos"
],
"hash": "8b3409faecdc902e17e07b6ca0073b1cf77ea86b"
},
"afterSource": {
"type": "dir",
"filePath": "../../packages/expo-modules-core",
"reasons": [
"expoAutolinkingIos",
"expoAutolinkingAndroid",
"expoAutolinkingIos"
],
"hash": "e29780a14f0893d8077140da997b3a8efe61c949"
}
},
{
"op": "changed",
"beforeSource": {
"type": "dir",
"filePath": "../../packages/expo-ui/android",
"reasons": [
"expoAutolinkingAndroid"
],
"hash": "258385016726ec1163bf8ea1569fcca1dad0a892"
},
"afterSource": {
"type": "dir",
"filePath": "../../packages/expo-ui/android",
"reasons": [
"expoAutolinkingAndroid"
],
"hash": "460db4cad2570128bda3948b3ecab04c57f322f7"
}
}
]Generated by PR labeler 🤖 |
packages/expo-modules-core/android/src/compose/expo/modules/kotlin/views/ExpoComposeView.kt
Outdated
Show resolved
Hide resolved
packages/expo-modules-core/android/src/compose/expo/modules/kotlin/views/ComposeViewProp.kt
Outdated
Show resolved
Hide resolved
| @@ -114,3 +127,42 @@ abstract class ExpoComposeView<T : ComposeProps>( | |||
| super.addView(view, index, params) | |||
| } | |||
| } | |||
|
|
|||
| class ExpoViewComposableScope(val view: ComposeFunctionHolder<*>) { | |||
There was a problem hiding this comment.
Can we make the view private or at least internal?
There was a problem hiding this comment.
Not really, we use the EventDispatcher defined below in other modules, and it's inline so it needs to have view as public as well.
...ore/android/src/compose/expo/modules/kotlin/views/ModuleDefinitionBuilderComposeExtension.kt
Outdated
Show resolved
Hide resolved
...ore/android/src/compose/expo/modules/kotlin/views/ModuleDefinitionBuilderComposeExtension.kt
Outdated
Show resolved
Hide resolved
...ore/android/src/compose/expo/modules/kotlin/views/ModuleDefinitionBuilderComposeExtension.kt
Outdated
Show resolved
Hide resolved
...ore/android/src/compose/expo/modules/kotlin/views/ModuleDefinitionBuilderComposeExtension.kt
Outdated
Show resolved
Hide resolved
packages/expo-ui/android/src/main/java/expo/modules/ui/ExpoUIModule.kt
Outdated
Show resolved
Hide resolved
|
Hi there! 👋 I'm a bot whose goal is to ensure your contributions meet our guidelines. I've found some issues in your pull request that should be addressed (click on them for more details) 👇
|
Why
@lukmccall Suggested to move the compose integration to a more compose-like approach:
From:
View(BottomSheetView::class)To:
View("BottomSheetView") { props ->How
Introduced a new
ComposeFunctionHolderthat is used for all compose views that only have composables, no custom classes. Also handles setting props by copying data classes.For now requires DSL for events to whitelist them in RN, we should drop that requirement soon.
Test Plan
Tested in BareExpo.
Checklist
changelog.mdentry and rebuilt the package sources according to this short guidenpx expo prebuild& EAS Build (eg: updated a module plugin).