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

fix motify() type definition conflict for transition prop #369

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bryanmylee
Copy link

Moti takes over the transition prop to define transition configs, but this might conflict with existing transition props on the original component.

While the implementation simply ignores passing transition to the original component, the type definition tries to merge the type declaration for both props, which sometimes results in impossible type intersections.

This change removes that possibility by omitting any keyof MotiProps from the component's original prop definition.

Copy link

vercel bot commented Dec 13, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
moti ❌ Failed (Inspect) Dec 13, 2024 10:40am

@bryanmylee
Copy link
Author

This should also solve #343

@nandorojo
Copy link
Owner

Looks reasonable, I assume this fixed it for you?

@bryanmylee
Copy link
Author

It does sometimes but I keep running into a TypeScript limitation TS2590: Expression produces a union type that is too complex to represent.

I'm not sure what the best approach to resolving performance issues is.

@nandorojo
Copy link
Owner

What if you just manually omit transition?

@bryanmylee
Copy link
Author

It still causes the type to blow up in complexity. I can't say for sure, but I think using Omit on Props in general is causing the issue.

@nandorojo
Copy link
Owner

In my experience that wouldn't quite be what causes it to blow up, more likely the depth of transition...for example if we removed the generic on the transition prop I wonder what would happen

@nandorojo
Copy link
Owner

Also is this the latest TS version

@nandorojo
Copy link
Owner

Thanks for digging in btw

@bryanmylee
Copy link
Author

I'm not on the latest TypeScript but on a pretty late version 5.3.3.

@nandorojo
Copy link
Owner

I think the problem originates from newer react-native and react-native-reanimated versions. I wonder if upgrading those + TypeScript in the moti repo would fix it.

Wrapping other dependencies and their types can be tough...

@bryanmylee
Copy link
Author

When I have time, I'd be happy to explore this more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants