tweakpane / use-tweaks

🎛️ Tweak React components with Tweakpane
https://codesandbox.io/s/use-tweaks-example-58e02
MIT License
768 stars 23 forks source link

Fix `UseTweaksValues` type #40

Closed ivanross closed 3 years ago

ivanross commented 3 years ago

This pr is meant to solve the issue described in #38.

any is produced by Leaves because it has circular reference and it is generic (and has some //@ts-ignore too). Now it is lazily evaluated so typescript doesn't complain about its recursivity.

I also removed Prev type and D type parameter because it seems to be unnecessary.

dbismut commented 3 years ago

This makes me realise how little I know about TypeScript. @ivanross it would be cool if you had the time to discuss about some types on another similar project that we're setting up with GM. @gsimone do you think we could grant Ivan access to controls?

dbismut commented 3 years ago

@gsimone I'll let you submit the new version on npm?

ivanross commented 3 years ago

@dbismut Sure, I will be happy to help!

gsimone commented 3 years ago

@ivanross Sending you an invite right now! 👍

gsimone commented 3 years ago

v0.3.1 published 👍

ivanross commented 3 years ago

@gsimone invite received 👍

unphased commented 3 years ago

Hi @ivanross I've tested 0.3.0 and 0.3.1, and it seems that https://github.com/pmndrs/use-tweaks/issues/43 is caused by this change.

Unfortunately I do not understand the code. Could you try to explain what changed? As far as I can tell on 0.3.0 the behavior is correct. It will produce a type that matches the type you specify. In 0.3.1, the type given is { "": <the type>} instead. This is wrong because it causes a typescript error, and on 0.3.1 we have to cast it as any and use it the same as before, which means the type is definitely wrong. I mean, an empty string key is also definitely wrong.

dbismut commented 3 years ago

@ivanross if you're still handling this, have a look at: https://github.com/pmndrs/leva/blob/bbbb3f9f33f576491543fb077d6b9c120737b710/packages/leva/src/types/public-types.ts#L152

@unphased you may want to try https://github.com/pmndrs/leva/ instead of use-tweaks. The API is still moving but it should be easy to switch to it.

unphased commented 3 years ago

@dbismut Thank you for the link! Very helpful. I will evaluate.

unphased commented 3 years ago

I'm now finding that 0.3.0 still suffers from this problem, so the type issue appears to be from a peer dependency. Unfortunately. Will post back if I find out what it is.

dbismut commented 3 years ago

Hey, I'm afraid use-tweaks won't be under active development now that Leva is getting traction. Did you try it yet?

unphased commented 3 years ago

Yep I am aware, yeah sorry I didnt have time to delve into it & maybe help with fixes. when i tried it in my project I got very mysterious type errors. It may be better now, since we are on react 17 now... but I've built some hooks off of use-tweaks by now so i'll have to revisit in just some weeks or months.