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

make typings of children/render prop optional to allow composition #137

Closed
alidcast opened this issue Jul 12, 2018 · 7 comments
Closed

make typings of children/render prop optional to allow composition #137

alidcast opened this issue Jul 12, 2018 · 7 comments

Comments

@alidcast
Copy link

alidcast commented Jul 12, 2018

Right now you either have to have a render or children prop

export type ValueProps<T> =
  | { initial: T; onChange?: ValueChange<T>; render: ValueRender<T> }
  | { initial: T; onChange?: ValueChange<T>; children: ValueRender<T> }

So you can't do this:

const Container = adopt({
  state: <State initial={{ foo: '' }} />,
})
@TrySound
Copy link
Collaborator

Yep composition with types sucks. But I wouldn't like to provide leaky types to solve this problem.

@alidcast
Copy link
Author

It worked with react-apollo, so maybe use the types they use?

@TrySound
Copy link
Collaborator

I don't see they use anything special.

@pedronauck could you clarify how adopt can work with apollo and not work with powerplug?

@alidcast
Copy link
Author

Ah you're right, they basically enforce children prop as well

 children: (result: QueryResult<TData, TVariables>) => React.ReactNode;

I was wrapping my queries/mutations which is why the error wasn't appearing

Hmm I wonder if anything can be done about this

@TrySound
Copy link
Collaborator

React team works on this problem. They probably will suggest a custom syntax like this, so type systems could solve the problem on syntax level.

const counter = adopt <plug.Value initial={0} />

@pedronauck
Copy link
Collaborator

pedronauck commented Jul 12, 2018

We don't care about the component type definition, this should be a components concern. When something like this happens to me, usually I change the children to be an optional type:

children?: (RenderProps: WhateverType) => React.ReactNode

So, of course, this can be leaky for powerplug!

What you can do is use the render prop on mapper:

const Composed = adopt({
  toggle: ({ render }) => <Toggle>{render}</Toggle>
})

@alidcast
Copy link
Author

Ok, so wrapping them or passing render prop is way to go given limitations of typings

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

No branches or pull requests

3 participants