software-mansion / react-native-reanimated

React Native's Animated library reimplemented
https://docs.swmansion.com/react-native-reanimated/
MIT License
8.85k stars 1.29k forks source link

Potential TypeScript errors in react-native-reanimated 3.4.0 #4645

Closed tjzel closed 10 months ago

tjzel commented 1 year ago

Description

This issue is dedicated for all react-native-reanimated users that have upgraded to 3.4.0. We made a significant change in which we remove old, manually typed definition file react-native-reanimated.d.ts to use automatically generated .d.ts files.

However, the code base is complex enough that full transition could not be done all at once and some type errors are to be expected. Therefore, please report your type problems in this issue, so we could have them all in one place.

Moreover, if issues that emerged are too burdensome for you to keep up smooth development with react-native-reanimated we've prepared a quick guide on how to revert those changes, until the fixes come.

This is a special branch that is the current version of react-native-reanimated but doesn't use automatically generated types. Keep in mind, that this version is also not type errors free and we don't plan on improving it, since its maintenance is too time consuming.

How to restore previous typings in react-native-reanimated

CLI solve

cd <path to node_modules> ; \
cd react-native-reanimated ; \
rm package.json ; \
wget https://raw.githubusercontent.com/software-mansion/react-native-reanimated/restore-d.ts/package.json ; \
wget https://raw.githubusercontent.com/software-mansion/react-native-reanimated/restore-d.ts/react-native-reanimated.d.ts

Manual solve

  1. Open our special branch.
  2. From there download two files:
    • package.json (make sure it's the file that's in the root of repository)
    • react-native-reanimated.d.ts
  3. Move those files (replace package.json) in node_modules/react-native-reanimated in your project files.
  4. In case TypeScript has trouble resolving types after that, a restart of your IDE/TypeScript compiler might be necessary.

Steps to reproduce

  1. Upgrade react-native-reanimated to 3.4.0.

Reanimated version

3.4.0

Platforms

Android, iOS, Web

aliza-khu commented 1 year ago

@tjzel, Any estimation time for release 3.4.0? And is the resolution of this issue going to be included in it or not?

renchap commented 1 year ago

I tried the nightly package in our app, and I am seeing one issue:

We store some shared values in a context, but the context default values are not proper shared values objects, as (afaik) you can not create share values outside of a component.

We initialize them with { value: 0 } objects, which are never used as those context values are always set when rendering the provider.

Example:

export const myContext = createContext<{ counter: SharedValue<number> }>({
  counter: { value: 0 },
})

// Then used like this:

const Component: React.FC = () => {
  const counter = useSharedValue(0)

  return (
    <myContext.Provider value={{ counter }}>
      {/* … */}
    </myContext.Provider>
  )
}

This fails with the following error:

index.tsx:76:3 - error TS2739: Type '{ value: number; }' is missing the following properties from type 'SharedValue<number>': addListener, removeListener, modify
tjzel commented 1 year ago

Hi @renchap. What you're describing is an issue stemming from improper typing in manually written types file. Out of simplicity SharedValue type was declared as an object with only value prop. This wasn't true though and SharedValue has its other properties 'revealed' now. Assigning an object that has only value prop is correctly detected as an error.

From my personal experience I also found default values for useContext a pain. I'd suggest the following:

export const myContext = createContext<{ counter: SharedValue<number> }>({
  counter: undefined as unknown as SharedValue<number>,
})

I know it's not pretty, but you could make it better:

interface myContextType {
  counter: SharedValue<number>;
}

export const myContext = createContext<myContextType>(
  {} as myContextType
);

Which I think is much more elegant.

tjzel commented 1 year ago

@aliza-khu About the release, I believe it may require a few additional weeks. With regards to the issue you raised, we will make every effort to fix it before the release, provided that we have sufficient time. If you have any ideas for a desired fix, we would be delighted if you could open a pull request.

Lastly, I kindly request that you avoid including non-TypeScript problems in this issue to ensure that all matters are handled systematically.

jblarriviere commented 1 year ago

Hello ! I tried the nightly package in our app as well, and I've encountered an issue when trying to provide an adapter created through createAnimatedPropAdapter as a 3rd argument to useAnimatedProps.

Example:

This example from the docs would raises the same error we have : https://reanimated-beta-docs.swmansion.com/docs/core/useAnimatedProps#adapters-

It fails with the following error:

TS2345: Argument of type 'AdapterWorkletFunction' is not assignable to parameter of type 'PropsAdapterFunction | PropsAdapterFunction[] | null | undefined'

Please let me know if there's more I can do !

tjzel commented 1 year ago

@jblarriviere Thanks for pointing that out! Adapters are quite complex and so far I had to cast it to old .d.ts types but missed some places - but thanks to your quick feedback a fix is already there!

robwalkerco commented 1 year ago

@tjzel After upgrading to 3.4.0 from 3.3.0 I'm getting the following type error:

node_modules/react-native-reanimated/src/reanimated2/threads.ts: /<redacted>/app/node_modules/react-native-reanimated/src/reanimated2/threads.ts: Unknown node type: "TSInstantiationExpression"

I've reviewed the latest installation instructions and updated versions of Babel etc. but the error persists.

casperolesen commented 1 year ago

@tjzel After upgrading to 3.4.0 from 3.3.0 I'm getting the following type error:

node_modules/react-native-reanimated/src/reanimated2/threads.ts: /<redacted>/app/node_modules/react-native-reanimated/src/reanimated2/threads.ts: Unknown node type: "TSInstantiationExpression"

I've reviewed the latest installation instructions and updated versions of Babel etc. but the error persists.

Same here

tjzel commented 1 year ago

Hi @robwalkerco @casperolesen! Is this error coming from babel? It seems like it. I just built a fresh RN app with Reanimated and it works for me - could you post more details about the error you are encountering?

robwalkerco commented 1 year ago

Is this error coming from babel? It seems like it. I just built a fresh RN app with Reanimated and it works for me - could you post more details about the error you are encountering?

I searched for TSInstantiationExpression in the reanimated code base, and it's not there, so it seems that the error must be coming from somewhere else. Unfortunately, there is no stack trace logged, just the error exactly as I included above.

I'm using RN 0.71.11, so I'll test building a fresh RN 0.71.11 app and adding Reanimated 3.4.0 tomorrow and see what happens.

robwalkerco commented 1 year ago

I resolved the TSInstantiationExpression issue by deleting yarn.lock and regenerating it. I was not able to find the specific dependency causing the issue.

casperolesen commented 1 year ago

Hi @robwalkerco @casperolesen! Is this error coming from babel? It seems like it. I just built a fresh RN app with Reanimated and it works for me - could you post more details about the error you are encountering?

I also tried testing in a fresh RN 0.72.3 build with Reanimated 3.4.0 with no errors. Then I added all the dependencies from my project to this fresh build - still no errors.

So then I tried to delete the yarn.lock in my project - and it resolved the Unknown node type: "TSInstantiationExpression"

tjzel commented 1 year ago

It's weird because yarn.lock is supposed to be auto-generated but funny things is we also had a case in the past in our repo that yarn.locks got corrupted somehow and I had to delete them and let them generate from scratch. Thanks for troubleshooting this problem (it would be a pain to get to it I guess since the error was so unspecific).

robwalkerco commented 1 year ago

Correction - adding the following to package.json resolutions seems to have resolved the issue for me:

"resolutions": {
    "@babel/plugin-transform-typescript": "^7.20.0",
    "@babel/types": "^7.20.0",
    ...
  },
hirbod commented 1 year ago

I can confirm that deleting yarn.lock indeed fixed the issue for us as well. Thanks for the pointer!

bfricka commented 1 year ago

The main issue I have with these changes is that not enough structural types are exported. Previously we had stuff like WithSpringConfig and WithTimingConfig. These types that describe configuration interfaces for various functions are very useful for composing helpers and predefined animations.

With these changes I now have to type them manually or extract them from the functions:

import { withTiming } from 'react-native-reanimated'

type TimingConfig = Parameters<typeof withTiming>['1']

Most of these types should be exported. Things like EasingFn, EasingFactoryFn, and all similar types should be exported. The only things that shouldn't be exported are the types that describe internal details. I'm actually really surprised that so few types are being exported that are associated with the public API of reanimated.

tjzel commented 1 year ago

Hi @bfricka. You're absolutely right.

This pull request is not supposed to be a breaking change - it shouldn't change our API. I should've explained this more elaborately in this issue's description, but types are not supposed to be gone in 3.4.0 - therefore, if a type you imported from react-native-reanimated is missing then it's an error and you are more than welcome to report it - and I will restore it.

Reason for some types missing is the fact that old .d.ts contained actual errors and imports that couldn't be resolved - therefore I decided to actually get as little from it as possible; also our codebase is quite complex and it's sometimes hard to tell if some piece of code is relevant (especially when its types are incorrect).

Those structural types you mention are an obvious mistake on my side - I focused on getting exported, auto-generated types right and didn't think about the fact that some important types might actually not be exported in our .ts code.

I will go through the types from our old .d.ts once again and add all missing, relevant types. Thank you for pointing this out!

diegolmello commented 1 year ago

We got around this issue with this patch https://github.com/RocketChat/Rocket.Chat.ReactNative/pull/5128/commits/9f93bc3543a63fa7484387c0c113c150f8967c6f

diff --git a/node_modules/react-native-reanimated/package.json b/node_modules/react-native-reanimated/package.json
index 0ffa9d3..00d6cd1 100644
--- a/node_modules/react-native-reanimated/package.json
+++ b/node_modules/react-native-reanimated/package.json
@@ -34,8 +34,8 @@
   },
   "main": "lib/module/index",
   "module": "lib/module/index",
-  "react-native": "src/index",
-  "source": "src/index",
+  "react-native": "lib/module/index",
+  "source": "lib/module/index",
   "types": "lib/typescript/index",
   "files": [
     "Common/",
tjzel commented 1 year ago

@diegolmello What issue do you mean?

diegolmello commented 1 year ago

@tjzel https://github.com/software-mansion/react-native-reanimated/issues/4645#issuecomment-1649993021

tjzel commented 1 year ago

With this patch you are forcing RN to use emitted ESModule instead of source code. This issue stems from some yarn.lock corruption. Try to delete your node_modules or node_modules and yarn.lock and see if the issue is gone without applying this patch.

diegolmello commented 1 year ago

Try to delete your node_modules or node_modules and yarn.lock and see if the issue is gone without applying this patch.

It didn't work.

tjzel commented 1 year ago

It's weird, in this directory we have source TS files and there shouldn't be any error resolving them, somehow the transpiler must be taking it as ESModule location or something. Perhaps your repo is misconfigured? You can also try deleting your yarn's cache.

GleidsonDaniel commented 1 year ago

With this patch you are forcing RN to use emitted ESModule instead of source code. This issue stems from some yarn.lock corruption. Try to delete your node_modules or node_modules and yarn.lock and see if the issue is gone without applying this patch.

If I'm using the library and it already has the types, why would I want to use source code instead of compiled code? (Which would even avoid all this problem with types)

tjzel commented 1 year ago

That's a fair question that I'm not sure I know an answer to at the moment. Probably depends on your configuration - if you need to compile source code to a form that suits your project.

natew commented 1 year ago

Releasing a breaking change in a patch (!!) version is causing us a lot of headache:

https://github.com/tamagui/tamagui/issues/1363#issuecomment-1671234125

We have to now replace all these types with our own manual types because otherwise we can’t support both versions.

bfricka commented 1 year ago

Releasing a breaking change in a patch (!!) version is causing us a lot of headache:

tamagui/tamagui#1363 (comment)

We have to now replace all these types with our own manual types because otherwise we can’t support both versions.

These are not breaking changes. None of the interfaces are different, only the types. We cannot expect authors to guarantee perfect TypeScript back compatibility, because there are just too many permutations of how user land can interact with and use types. There's no way to guarantee that a type change doesn't break someone's usage.

That said, I agree that primary interface types should have been exported. Frankly, it's not a hard thing to address, and you're you still get to benefit from someone else's hard work.

Since you did all the work of replacing the previously exported types with your own, why not open a PR that exports the types you need? That would benefit everyone.

tjzel commented 1 year ago

@natew This was not on purpose. Accidentally WithDecayConfig was imported as import instead of import type and for some reason that import was not detected by TypeScript as type import and was added to JS code and that broke web. Relevant fix has already been merged and we are preparing 3.5.0 that will have this and other fixes included. If you need it asap I suggest installing react-native-reanimated's nightly version (yarn add react-native-reanimated@nightly or npm install react-native-reanimated@nightly).

natew commented 1 year ago

@bfricka removing an exported type export is a breaking change, that said I am one of the biggest proponents of Reanimated and the Software Mansion team and my comment was only made out of concern. I also haven't replaced the types yet myself, but in any case I'd need official word from the team on what they'd want to do before I made any contribution.

@tjzel thanks! Look forward to it.

efstathiosntonas commented 1 year ago

leaving this here since the other issue is closed:

@tjzel this issue is back on 0.72.4, offending(~?~) commit: https://github.com/facebook/react-native/commit/2558c3d4f56776699602b116aff8c22b8bfa176a

using: 3.5.0-nightly-20230813-f55e831b1

edit: after removing | string from react-native StyleSheetTypes.d.ts` all errors went away

Screenshot 2023-08-14 at 13 44 03

tjzel commented 1 year ago

@efstathiosntonas Yeah, after I 'fixed' this error, and then 'fixed' it again I realised there are more entry points for it to break, so I decided to finally rewrite and fix styles of Animated Components.

If you want (I'd appreciate it) I can build a tarball from that branch so you could check if it fixes your issues - I think it should, since I added a bunch of type tests and it's passing them.

efstathiosntonas commented 1 year ago

@tjzel sure, post the link when ready!

Crare commented 1 year ago

after metro bundle load I get this error:


 BUNDLE  ./index.js 

error: node_modules/react-native-reanimated/src/reanimated2/threads.ts: <project-folder>/node_modules/react-native-reanimated/src/reanimated2/threads.ts: unknown node of type "TSInstantiationExpression" with constructor "Node"

using react-native-reanimated 3.4.2

EDIT: ok it got fixed by removing node_modules-folder and yarn.lock-file.

tjzel commented 1 year ago

@efstathiosntonas https://github.com/software-mansion/react-native-reanimated/blob/%40tjzel/tarball/types/animated-style/react-native-reanimated-3.4.9.tgz, preferably download the file into your project and use yarn add file:./react-native-reanimated-3.4.9.tgz, don't mind the version, it's just to differentiate from others since if you use 3.4.0 it could try to install it from cache instead of this tarball. Thanks and let me know about TS errors you encounter!

devoren commented 1 year ago

When i use useAnimatedRef<Reanimated.ScrollView>(); i get this type error (style error due to ...rest prop that extends ScrollViewProps):

index.d.ts(154, 9): The expected type comes from property 'ref' which is declared here on type 'IntrinsicAttributes & IntrinsicClassAttributes<AnimatedScrollViewClass> & Readonly<AnimateProps<AnimatedScrollViewProps>>'

helperTypes.d.ts(32, 5): The expected type comes from property 'style' which is declared here on type 'IntrinsicAttributes & IntrinsicClassAttributes<AnimatedScrollViewClass> & Readonly<AnimateProps<AnimatedScrollViewProps>>'

image
AntQwanlity commented 1 year ago

still having unknown node of type "TSInstantiationExpression" with constructor "Node" issue after deleting node_modules and yarn.lock, anyone else ?

thanksyouall commented 1 year ago

@efstathiosntonas Yeah, after I 'fixed' this error, and then 'fixed' it again I realised there are more entry points for it to break, so I decided to finally rewrite and fix styles of Animated Components.

If you want (I'd appreciate it) I can build a tarball from that branch so you could check if it fixes your issues - I think it should, since I added a bunch of type tests and it's passing them.

After than I have a new error: export 'WorkletRuntime' (reexported as 'WorkletRuntime') was not found in './core' (possible exports: configureLayoutAnimations, configureProps, createWorkletRuntime, enableLayoutAnimations, getViewProp, initializeSensor, isConfigured, isReanimated3, makeMutable, makeRemote, makeShareable, registerEventHandler, registerSensor, runOnJS, runOnUI, startMapper, stopMapper, subscribeForKeyboardEvents, unregisterEventHandler, unregisterSensor, unsubscribeFromKeyboardEvents) What could be the reason?

tjzel commented 1 year ago

Hi @thanksyouall @AntQwanlity . It seems upgrading Reanimated in your project has failed and not all the files got updated. I suggest you remove your node_modules and do npm cache clean or yarn cache clean. If this didn't help, remove your package-lock.json or yarn.lock and repeat the previous steps.

tjzel commented 1 year ago

@devoren I've been working on fixing the typings of styles in Animated Components, could you try to install our nightly version and see if the error is gone? yarn add react-native-reanimated@nightly

Xmaxer commented 1 year ago

This is probably related so I thought I'd throw this in here:

 const animatedInputWidthStyle = useAnimatedStyle<ViewStyle>(() => {
    const nextWidth = `${interpolate(
      expandingAnimation.value,
      [0, 1],
      [30, 100],
    )}%`;

    return {
      width: nextWidth,
    };
  });

produces:

TS2345: Argument of type '() => { width: string; }' is not assignable to parameter of type '() => ViewStyle'.
Call signature return types '{ width: string; }' and 'ViewStyle' are incompatible.
The types of 'width' are incompatible between these types.
Type 'string' is not assignable to type 'DimensionValue | undefined'.

But assigning it to ViewStyle directly definitely works:

  const x: ViewStyle = {
    width: '100%',
  };

produces no errors.

I also tried out the latest nightly build, to no avail.

tjzel commented 1 year ago

@Xmaxer what you presented are not the same use cases and the underlying problem is a common pitfall in TypeScript.

Consider the following snippet (notice it doesn't use Reanimated):

function variableFoo() {
  return '100%';
}

function constantFoo() {
  return '100%' as const;
}

const variableWidth = `${variableFoo()}`;

const constantWidth = `${constantFoo()}` as const;

const castWidth = `${constantFoo()}` as `${number}%`;

const style1: ViewStyle = {
  width: variableWidth, // this produces an error
};

const style2: ViewStyle = {
  width: constantWidth, // this is ok
};

const style3: ViewStyle = {
  width: castWidth, // this is ok
};

This happens because TypeScript always tries to infer the type as wide as possible - in this case it infers a computed string as string what causes errors. It's because width property of ViewStyle is not a string but: Screenshot 2023-09-04 at 12 02 32 The keyword as const tells TypeScript to do the opposite: infer the type as narrowly as possible.

If you're not convinced, here's your example that I modified to highlight the problem:

const wrongNextWidth = `${interpolate(
  expandingAnimation.value,
  [0, 1],
  [30, 100]
)}%`;

const correctNextWidth = `${interpolate(
  expandingAnimation.value,
  [0, 1],
  [30, 100]
)}%` as const;

const wrongStyle: ViewStyle = {
  width: wrongNextWidth, // this produces an error
};

const correctStyle: ViewStyle = {
  width: correctNextWidth, // this is ok
};

So your best bet here it to use as const in those situations - or if it's still problematic for TypeScript, try to cast it to it's direct type (${number}% here).

Xmaxer commented 1 year ago

@tjzel Ah I see, my fault for not checking the true definition of DimensionValue. Thanks for the clarification and explanation, much appreciated.

AuroPick commented 1 year ago

Hi when i try to import something. Intellisense suggests lib/typescript instead library image

my tsconfig.json

{
  "extends": "@tsconfig/react-native/tsconfig.json"
}
devoren commented 1 year ago

@tjzel Hello! Unfortunately, it did not help, the same error: Type 'AnimatedRef<AnimatedScrollView>' is not assignable to type 'LegacyRef<AnimatedScrollViewClass> | undefined'. And Type 'false | ViewStyle | { flex: number; } | RegisteredStyle<ViewStyle> | RecursiveArray<ViewStyle | Falsy | RegisteredStyle<ViewStyle>> | null' is not assignable to type 'StyleProp<AnimateStyle<MaybeSharedValue<StyleProp<ViewStyle>>>>'.

tjzel commented 1 year ago

@devoren I think that when I told you to use nightly relevant fix wasn't actually in the nightly yet, I apologise for that. If it didn't work a few days back please check it again. If the error is actually from today, then I think something went wrong with the upgrade, as style prop being of type StyleProp<AnimateStyle<MaybeSharedValue... is no longer the case right now.

Screenshot 2023-09-06 at 09 17 31 As you can see, there are no errors with more or less the use case you posted.

tjzel commented 1 year ago

@AuroPick This is unfortunately something we cannot do anything about. VSCode scans our library recursively and suggests to import some internal objects - it's a known issue (thanks @satya164 for pointing me to it). Fortunately there is a solution. All you need to do is to add to auto-import ignored directories **/react-native-reanimated/lib.

devoren commented 1 year ago

@tjzel hey, thank you, with {...rest} no porblem but there are other errors (same use case): 1) still style prop is not working (without ref):

image

2) ref error (without style): const scrollViewAnimatedRef = useAnimatedRef<Reanimated.ScrollView>(); I dont know why my ref does not work

image

react-native-reanimated@3.5.0-nightly-20230905-8cecbd054

tjzel commented 1 year ago

@devoren There is still something wrong with your version - e.g. type AdaptTransforms is no longer used in Animated Styles. Try to remove your node_modules, then install the packages again. After that please use npm why react-native-reanimated or yarn why react-native-reanimated to see if you actually only have the nightly version installed.

devoren commented 1 year ago

@tjzel Thank you! You were right, after removing node_modules and reinstalling everything works!

doomsower commented 1 year ago

I'm getting this error with react-native-reanimated@3.5.2:

export 'WorkletRuntime' (reexported as 'WorkletRuntime') was not found in './core' (possible exports: configureLayoutAnimations, configureProps, createWorkletRuntime, enableLayoutAnimations, getViewProp, initializeSensor, isConfigured, isReanimated3, makeMutable, makeRemote, makeShareable, makeShareableCloneRecursive, registerEventHandler, registerSensor, runOnJS, runOnUI, startMapper, stopMapper, subscribeForKeyboardEvents, unregisterEventHandler, unregisterSensor, unsubscribeFromKeyboardEvents)
ERROR in ../../node_modules/react-native-reanimated/lib/module/reanimated2/index.js 2:0-188

It appears that js file node_modules/react-native-reanimated/lib/module/reanimated2/index.js contains following line:

export { runOnJS, runOnUI, createWorkletRuntime, WorkletRuntime, makeMutable, makeShareableCloneRecursive, isReanimated3, isConfigured, enableLayoutAnimations, getViewProp } from './core';

But node_modules/react-native-reanimated/lib/module/reanimated2/core.js does not export WorkletRuntime.

That's because it's exported as a type from node_modules/react-native-reanimated/src/reanimated2/core.ts:

export type { WorkletRuntime } from './runtimes';

Temporary reverting to v 3.3.0 fixes this issue