Skip to content

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

Open
bryanmylee wants to merge 1 commit into
nandorojo:masterfrom
bryanmylee:master
Open

fix motify() type definition conflict for transition prop#369
bryanmylee wants to merge 1 commit into
nandorojo:masterfrom
bryanmylee:master

Conversation

@bryanmylee

Copy link
Copy Markdown

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.

@vercel

vercel Bot commented Dec 13, 2024

Copy link
Copy Markdown

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
Copy Markdown
Author

This should also solve #343

@nandorojo

Copy link
Copy Markdown
Owner

Looks reasonable, I assume this fixed it for you?

@bryanmylee

Copy link
Copy Markdown
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
Copy Markdown
Owner

What if you just manually omit transition?

@bryanmylee

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Owner

Also is this the latest TS version

@nandorojo

Copy link
Copy Markdown
Owner

Thanks for digging in btw

@bryanmylee

Copy link
Copy Markdown
Author

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

@nandorojo

Copy link
Copy Markdown
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
Copy Markdown
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