karet icon indicating copy to clipboard operation
karet copied to clipboard

Provider to support children with observables

Open rikutiira opened this issue 5 years ago • 1 comments

https://codesandbox.io/s/yw63o1zr1x

Here's an example case where you need to explicitly use <React.Fragment> to wrap children inside <Provider> because one of the children is an observable (due to U.when(obs, value)).

It'd be preferable if karet's Provider would work out of the box with observable children.

Another thing here is that for some reason React is warning about missing keys when using children inside the React.Fragment. I think there has been issues with same thing previously but those were fixed?

rikutiira avatar Oct 04 '18 11:10 rikutiira

Yes, the current behaviour and workaround is mentioned in the README:

If you need to have observable children inside the provider, you can wrap the children inside a Fragment.

It should be possible to wrap createContext to return a Provider whose children, but not other properties, are lifted. Aside from the additional implementation complexity, I think I didn't do that, because that would be different from the way lifting otherwise works. Thinking about this now, I'm not yet sure whether special case lifting of children should be done in this case. As you say, it would usually do the right thing, but, OTOH, it would be a special case and as such might be surprising.

Another thing here is that for some reason React is warning about missing keys when using children inside the React.Fragment.

Hmm... That seems to be caused by having both a non-observable child and an observable child. You can e.g. explicitly give a key for the observable child <Child key="something"/> or wrap the observable inside a span or div to silence the warning. React automatically assigns keys in a static list of elements, but in this case the list is not static. Perhaps it is possible to work around this inside Karet. I'm not immediately sure.

BTW, note that you can also write <>...</> instead of <React.Fragment>...</React.Fragment> in the Sandbox. (Works when you have Babel 7 or TS, for example.)

polytypic avatar Oct 07 '18 22:10 polytypic