Conversation
|
The Pull Request introduced fingerprint changes against the base commit: 323acc7 Fingerprint diff[
{
"op": "changed",
"beforeSource": {
"type": "dir",
"filePath": "../../packages/expo-ui/android",
"reasons": [
"expoAutolinkingAndroid"
],
"hash": "11d9d951b8df5128281c9229e323c88e8113b0e7"
},
"afterSource": {
"type": "dir",
"filePath": "../../packages/expo-ui/android",
"reasons": [
"expoAutolinkingAndroid"
],
"hash": "69b9a75f3920b4da4e783ce887f135fb33be4ec4"
}
}
]Generated by PR labeler 🤖 |
packages/expo-modules-core/android/src/main/cpp/types/FrontendConverter.cpp
Outdated
Show resolved
Hide resolved
1a90cc6 to
3568e22
Compare
…o/modules/kotlin/views/ComposeViewProp.kt
Co-authored-by: Expo Bot <34669131+expo-bot@users.noreply.github.com>
|
Subscribed to pull request
Generated by CodeMention |
Kudo
left a comment
There was a problem hiding this comment.
promising work! mostly great and the only comment is to have the createModifiers helper. then we can merge it. let's taste the modifiers implementation between SharedRef and array after merge
| systemImage, | ||
| onButtonPressed: onPress, | ||
| // @ts-expect-error | ||
| modifiers: props.modifiers?.map((m) => m.__expo_shared_object_id__), |
There was a problem hiding this comment.
| modifiers: props.modifiers?.map((m) => m.__expo_shared_object_id__), | |
| modifiers: createModifiers(props.modifiers), |
maybe create a helper like createModifiers that to prevent exposing the shared object internal __expo_shared_object_id__ and the @ts-expect-error everywhere
function createModifiers(modifiers: ExpoModifiers[] | null) {
// @ts-expect-error
return modifiers?.map((m) => m.__expo_shared_object_id__),
}There was a problem hiding this comment.
I thought about it but it's not modifier specific – we use it in more places/modules.
I'd be open to adding sharedObjectToProp to EMC – @lukmccall wdyt?
There was a problem hiding this comment.
if we were not moving it to sharedObjectToProp. the current modifiers?.map((m) => m.__expo_shared_object_id__) is not scalable for 3rd party modifiers. having the createModifiers helper in expo-ui would help a little at least
There was a problem hiding this comment.
hmm, we can support any modifiers like that, but anytime you write a component that accepts SO as a prop you need this magic line. We should fix this in core at some point...
There was a problem hiding this comment.
alright since you have some idea to solve the problem in mind, then it makes sense to me. we can follow up later in a separate pr
Why
Mirroring the same work for iOS (also the same API) by @Kudo.
How
Used SharedRefs and invoking JavaScriptFunctions for
clickable. This makes the modifiers reusable across different packages.Test Plan
Tested in BareExpo by placing modifiers on various components.
Checklist
changelog.mdentry and rebuilt the package sources according to this short guidenpx expo prebuild& EAS Build (eg: updated a module plugin).