pmndrs / valtio

🧙 Valtio makes proxy-state simple for React and Vanilla
https://valtio.dev
MIT License
9.16k stars 257 forks source link

DeepResolveType breaks React.ComponentType #292

Closed giladv closed 2 years ago

giladv commented 2 years ago

Hi,

useSnapshot uses DeepResolveType as the return type which breaks the type if one of the props is a ref to a ComponentType.

const store: {Comp: React.ComponentType} = {Comp: () => null}

function MyComponent(){
    const snap = useSnapshot(store) as typeof store;

    return <snap.Comp/>; // this works fine
}

function MyComponent(){
    const snap = useSnapshot(store);

    return <snap.Comp/>; // TS2604: JSX element type 'snap.Comp' does not have any construct or call signatures.
}
dai-shi commented 2 years ago

A component is not a serializable object, so you may want to use ref(). Hopefully, it solves the type issue. Anyway, DeepResolveType isn't an ideal solution and we would like to improve it in the future, maybe Awaited helps.

giladv commented 2 years ago

i am using ref but the problem is asRef does not seem to b exposed and when you define the interface of the state you cant specify the type is a ref of something. so:

this works fine because you let it infer the type, but it's not a solution because you want to define the type of your store, not infer it from a specific value.

const store = {Comp: ref(() => null)}

function MyComponent(){
    const snap = useSnapshot(store);

    return <snap.Comp/>; // this works fine
}

if you try to define it, it breaks

const store: {Comp: React.ComponentType} = {Comp: ref(() => null)}

function MyComponent(){
    const snap = useSnapshot(store);

    return <snap.Comp/>; // TS2604: JSX element type 'snap.Comp' does not have any construct or call signatures.
}

we need a Ref<T> sort of a utility type i believe so we can:

type Store = {
       Comp: Ref<React.ComponentType>
}
dai-shi commented 2 years ago

Thanks for the explanation. Yeah, this is asked before, but we avoid to expose AsRef because it might be changing in the future.

Does this work in your use case?

const store = proxy({
  Comp: ref<React.ComponentType>(() => null),
})

or

const store = proxy({
  Comp: ref((() => null) as React.ComponentType),
})

But, I now think your first workaround is good enough in the meantime:

const snap = useSnapshot(store) as typeof store
giladv commented 2 years ago

this works, but i have to infer the type from the value. which is not what i want

const store = proxy({
  Comp: ref<React.ComponentType>(() => null),
})

type Store = typeof store;

i want to define the type of my store then use it

type Store = {
    components: React.ComponentType[];
}

const store = proxy<Store>({
  components: []
})

while this works, it is not very clean..

const snap = useSnapshot(store) as typeof store
dai-shi commented 2 years ago

How about this? I didn't try it though.

// we expect this is eliminated with minification.
const dummyRef = ref<React.ComponentType>(() => null)
type RefComponentType = typeof dummyRef

type Store = {
  components: RefComponentType[]
}
Codpoe commented 2 years ago

How about this? I didn't try it though.

// we expect this is eliminated with minification.
const dummyRef = ref<React.ComponentType>(() => null)
type RefComponentType = typeof dummyRef

type Store = {
  components: RefComponentType[]
}

There is a TS error Exported variable 'dummyRef' has or is using name 'AsRef' from external module "node_modules/valtio/vanilla" but cannot be named.ts(4023)

dai-shi commented 2 years ago

🤦‍♂️

giladv commented 2 years ago

i'd rather use as typeof store anyways haha

dai-shi commented 2 years ago

Sorry for the inconvenience. Thought AsRef is neat, but not very developer friendly (it breaks like this). Let's look for a better solution.

dai-shi commented 2 years ago

While #333 doesn't solve the use case of this, it avoids using const enum which was the root cause of the problem. So, I will close this issue.

I still think the easy workaround is as typeof store.