Fix SharedValueType type

[Andarius]

Description

SharedValue type can cause errors when using a boolean as type for useSharedValue.

The following code will raise an error because sharedBool becomes of type Animated.SharedValue<false> | Animated.SharedValue<true> (because SharedValue<T extends SharedValueType> extends the type boolean as true | false) instead of Animated.SharedValue<boolean>

const sharedBool = useSharedValue<boolean>(false)
if(sharedBool.value)
  sharedBool.value = false

Changes

Updated react-native-reanimated-tests.tsx and react-native-reanimated.d.ts

Before

Raise an error: Type 'false' is not assignable to type 'true'.

After

Does not raise the error

Checklist

• Updated TS types
• Added TS types tests
• Ensured that CI passes

[terrysahaidak]

Im pretty sure the current type should work without <boolean> since I use it in myltiple projects already.

Could you please provide reproducing project for this one? I wonder perhaps there are some differences in our ts configs or so.

You check for boolean only here, but actually if it's the case then useSharedValue(0) should work the same way - you won't be able to assign anything but 0.

[Andarius]

Hey @terrysahaidak, if you update react-native-reanimated-tests.tsx with

function SharedValueTest() {
  const translate = useSharedValue(0);
  const translate2 = useSharedValue(0, true);
  const translate3 = useSharedValue(0, false);

  const sharedBool = useSharedValue<boolean>(false)

  if(sharedBool.value)
      sharedBool.value = false

  return <Animated.View style={styles.container} />;
}

and run tsc -p . in the react-native-reanimated project you should see an error.

Edit: my tsc version is 4.0.2

[terrysahaidak]

So you're fixing the case when you specify explicitly type for shared value?

[Andarius]

For both cases, it triggers the same error

[terrysahaidak]

Let me check tomorrow why I don't have this error in my gallery-toolkit project.

[Andarius]

Here is a repro with TS playground

[Andarius]

Hey @terrysahaidak, any update on this ?

[mrousavy]

@Andarius I'm having the same TS errors and your types work for me. 👍

[jakub-gonet]

At first glance, I wasn't happy with that solution because it effectively hardcodes booleans and if in the future we'd want to add another union type we'll have to hardcode it there as well.

I read some discussions about this in the TS repo and seems like this is a limitation of distributing over unions so we can't do much about this.

@wcandillon, maybe you can suggest something to make it less hacky?

[wcandillon]

As @terrysahaidak mentioned, I cannot reproduce the original issue. Could it be related to the typeScript version or could you provide me with a sample file where I could reproduce the issue?

[mrousavy]

@wcandillon have you tried the TS playground link @Andarius sent? Which TS version are you on? I can reproduce the issue in my project on v4.1.2

I find it rather weird that it doesn't abstract true or false to boolean, but it does work for any string-like type (e.g. something -> string). Maybe this is an actual typescript bug? @Andarius have you tried asking in the TS discord?

[Andarius]

@mrousavy, as mentioned by @jakub-gonet, it's a typescript limitation (see this issue for instance: microsoft/TypeScript#41333)

[wcandillon]

@mrousavy Thanks a lot!

Sorry if this was already explained in another thread but why not do:

export type SharedValue<T> = {
  value: T;
};


export function useSharedValue<T>(
  initialValue: T,
  shouldRebuild?: boolean
):  SharedValue<T>;


const bar = useSharedValue(false)
if(bar.value) {
  bar.value = false;
}

[Andarius]

So <T> could be any type ? Do you want to keep using RawSharedValue or is it something that can be ignored ?

[wcandillon]

I'm not sure what is the use case of RawSharedValue

[Andarius]

I have no idea too, but that would make it simpler here

[wcandillon]

@jakub-gonet @terrysahaidak green light to delete RawSharedValue?

[terrysahaidak]

This is more a question what cannot be a shared value. Perhaps @Szymon20000 or @karol-bisztyga can answer this question.

If we can pass just anything then yeah we can remove this constraint, otherwise type safety makes here no sense since we don't type safe anything which may lead to runtime errors.

[jakub-gonet]

In general, we can't store anything in the shared value (e.g. Map) so this should stay as it is. We should be able to use anything that is a value from JSI and those things are already listed.

[mrousavy]

@jakub-gonet unfortunately that's not so easy, as we have object in those types, and Map is an object. here's an example in the TS playground where I'm passing a Map to useSharedValue without any problems.

I think the simple approach @wcandillon suggested is our only option, and for stuff that's not supported (e.g. Map) we should just check those in debug builds and throw errors.

[jakub-gonet]

That's unfortunate. I was afraid we'll relax this type too much but object is sort of catch-all here.

I feel convinced, if @terrysahaidak doesn't have any further objections I think we can change it.

[terrysahaidak]

Should we check for undefined then? We can assign null, but can we assign undefined? because this constraint is not only for the thing we can pass to useSharedValue, but also for the thing we can assign to .value later.

But I still don't understand why we (me and @wcandillon) don't have such errors in our projects.

I guess something is wrong with our ts configs.

[wcandillon]

@terrysahaidak I think that we would have the same issue it's just that we don't have the exact same use case (boolean type that gets refined via if statement and then re-assigned in that if branch).

[mrousavy]

@terrysahaidak why should we check for undefined? ValueType::UndefinedType is as valid as any other JSI type. See this

[wcandillon]

one more argument for removing RawSharedValue is that it doesn't contain null which is geniuly helpful

[Andarius]

I pushed a new version with RawSharedValue removed

[wcandillon]

I'm hitting a similar issue by assigning an enum to a shared value. Removing RawSharedValue solves it. I can only push for this change 🤗

[jakub-gonet]

Sorry for the delay, I think we can merge that one.

@Andarius, could you resolve conflicts?