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

Add Map and Set support #1228

Open
likern opened this issue Sep 15, 2020 · 8 comments
Open

Add Map and Set support #1228

likern opened this issue Sep 15, 2020 · 8 comments
Labels
bug-bash-jan22 Issues visited during Bug Bash Jan 2022 Feature request 🏠 Reanimated 2

Comments

@likern
Copy link
Contributor

likern commented Sep 15, 2020

Description

This support is required to store information about pages (rows grouped by pages) for Animated.ScrollView.

Now, if I'm correct, there is no such support. And Map were treated as usual object (or was undefined) inside worklet, if was created through useSharedValue(new Map())

Caveats

Motivation

Track page ids and efficiently retrieve rows by this id (for adding or deleting rows on demand).

Also to use to store animated references.

@Andarius
Copy link
Contributor

Andarius commented Oct 6, 2020

Same issue that it took me some time to debug. If I'm defining a Map such that Map<string, Animated.SharedValue<number>> this will crash the app if I try to iterate over it (but not access it's size) in a worklet.

@andreialecu
Copy link
Contributor

andreialecu commented Feb 6, 2021

I attempted to use a plain object for this, and ran into some weird issues with rc.0.

const someValue = useSharedValue({})

Which would then be updated via someValue.value = {...someValue.value, [key]: newValue }.

However, when attempting to read someValue.value[key] from a scroll handler it would display stale values. Sometime it would contain the key I wanted and sometimes it didn't.

Actually, even attempting to read it immediately after setting it would show stale data:

someValue.value = {...someValue.value, [key]: newValue }
console.log(someValue.value) // key sometimes isn't here, after a while it may or may not be

This especially manifests when a lot of JS calls try to update the value so it may be some sort of race condition between the UI and JS thread.

I wasn't able to upgrade past rc.0, because even with rc.3 I'm running into _setGlobalConsole is undefined: #1640 (tried all the solutions in that thread, nuking caches, etc, without success)

@likern
Copy link
Contributor Author

likern commented Feb 6, 2021

@andreialecu Is this line is run in context of worklet?

someValue.value = {...someValue.value, [key]: newValue }

I think ... syntax is not supported inside running worklets. At least I had issue with destructuring array like this one

const [element, ...other] = array;

if this code was run inside of worklet, while on usual JS side it is worked as expected.

@andreialecu
Copy link
Contributor

By updating .value property. Updating .value is sync when running on the UI thread, or async when running on the React Native JS thread.

ref: https://docs.swmansion.com/react-native-reanimated/docs/shared-values#shared-values-vs-animatedvalue

I guess this would explain the behavior above in the general sense.

However I also tried wrapping the entire set call in runOnUI() without success 🤔
Will probably revisit this at a later point.

In the mean-time got unblocked by using normal react useState and passing it in the dep array to useAnimatedScrollHandler

@andreialecu
Copy link
Contributor

I think ... syntax is not supported inside running worklets. At least I had issue with destructuring array like this one

I tried Object.assign as well and the behavior persisted from within runOnUI. It seems like some sort of race condition exists. But again I'm on rc.0, so this may have been fixed in latest rcs.

You could try it yourself if you need map-like behavior because it seems to work. The problem in my case was when I started updating it too fast.

@jkadamczyk jkadamczyk added the bug-bash-jan22 Issues visited during Bug Bash Jan 2022 label Jan 28, 2022
@jkadamczyk
Copy link
Contributor

Hi @likern @andreialecu

Let me try to rephrase it.
What you want to have implemented in Reanimated 2 is support for using Map and Set JS objects as a value passed to SharedValue from Reanimated 2.

That would be a feature request we can accept, but I can't guarantee we will implement something like this.
Correct me if I'm wrong here

@jkadamczyk
Copy link
Contributor

@likern @andreialecu

From what I see apart from the feature request to support Map and Set you tried to work around this issue by using a JS Object {'key': 'value'} and found some issues with it. If you still have issues, could you please create a Bug report issue separately and provide a small repro with it if you still have problems with Reanimated 2 working as desired?

Thanks for sparing your time and have a nice day 😊

@jkadamczyk jkadamczyk added Close when stale This issue is going to be closed when there is no activity for a while and removed Close when stale This issue is going to be closed when there is no activity for a while labels Jan 28, 2022
@Latropos Latropos added the Close when stale This issue is going to be closed when there is no activity for a while label Aug 4, 2023
@thecynicalpaul
Copy link

Would love to see this happen as well.

@github-actions github-actions bot removed the Close when stale This issue is going to be closed when there is no activity for a while label Aug 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-bash-jan22 Issues visited during Bug Bash Jan 2022 Feature request 🏠 Reanimated 2
Projects
None yet
Development

No branches or pull requests

6 participants