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

DeepResolveType breaks React.ComponentType #292

Closed
giladv opened this issue Dec 4, 2021 · 10 comments · Fixed by #333
Closed

DeepResolveType breaks React.ComponentType #292

giladv opened this issue Dec 4, 2021 · 10 comments · Fixed by #333
Labels
enhancement New feature or request

Comments

@giladv
Copy link

giladv commented Dec 4, 2021

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

dai-shi commented Dec 4, 2021

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

giladv commented Dec 4, 2021

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

dai-shi commented Dec 4, 2021

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

giladv commented Dec 4, 2021

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

dai-shi commented Dec 4, 2021

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

Codpoe commented Dec 4, 2021

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

dai-shi commented Dec 4, 2021

🤦‍♂️

@giladv
Copy link
Author

giladv commented Dec 4, 2021

i'd rather use as typeof store anyways haha

@dai-shi
Copy link
Member

dai-shi commented Dec 4, 2021

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

dai-shi commented Jan 21, 2022

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.

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

Successfully merging a pull request may close this issue.

3 participants