Typed children for function-based widgets#544
Conversation
…ccept it being passed from .childre()
| import { RegistryHandler } from './RegistryHandler'; | ||
|
|
||
| export type OmitProp<T, K extends keyof any> = Pick<T, Exclude<keyof T, K>>; | ||
| export type Omit<T, K extends keyof T> = Pick<T, Exclude<keyof T, K>>; |
There was a problem hiding this comment.
will this conflict with the global Omit type added in 3.5? I presume not but worth a check
There was a problem hiding this comment.
It shouldn't do, I did a quick check and there no complaints about Omit
| children?: W['children'] | ||
| children?: any | ||
| ): WNode<W> { | ||
| if ((properties as any).children) { |
There was a problem hiding this comment.
is this correct now? would it not be __children__?
There was a problem hiding this comment.
Fixed and a couple of other misses too.
| assert.isTrue(consoleWarnStub.calledOnce); | ||
| }); | ||
|
|
||
| it('typed children', () => { |
There was a problem hiding this comment.
do we need an example test with w() also?
| assert.deepEqual(adapter.getChildren(w(WidgetBase, {}, [v('span')])), [v('span')]); | ||
| assert.deepEqual(adapter.getChildren(w(WidgetBase, {}, [w(WidgetBase, {})])), [w(WidgetBase, {})]); | ||
| assert.deepEqual(adapter.getChildren(w(WidgetBase, {})), []); | ||
| assert.deepEqual(adapter.getChildren(w(WidgetBase, {})), undefined); |
There was a problem hiding this comment.
how come this had to change?
There was a problem hiding this comment.
Because by default children are not set to an empty array when using w any more.
There was a problem hiding this comment.
but this is the assertion for getChildren no? which is both typed as an array and returns an empty array if not set?
There was a problem hiding this comment.
If it is a WNode or VNode then it just returns the children from the element.
There was a problem hiding this comment.
Ah okay, sorry I missed that this was in the selector, and not the assertion template. This makes me concerned about these types/values a little bit though.
I think any kind of children interface should be consistently returning either an empty array [] for no children, an array of one ['a'] for one item, or an array of many ['a','b','c'] for many items. This means the end user can consistently work on an array without guards for api's such as this, and would match other child like api's such as NodeList.
I'm not sure what the ramifications are on types or whether it's possible with TSX, but it'd seem more consistent to make the user define create().children<[ whatever ]>() over create().children<whatever>() if they need to define a single item.
There was a problem hiding this comment.
I think that's fair point about always having children and it being an array (that can be defaulted). I've made some changes (WIP atm as need to put back the defaulting of children) that added support for children to be mandatory and able to be typed singularly but are consumed by the widget author as the first item in an array.
| }; | ||
|
|
||
| function mockIntersection(): MiddlewareResult<any, any, any>; | ||
| function mockIntersection(): MiddlewareResult<any, any, any, any>; |
There was a problem hiding this comment.
can we add a convenience type for this? it's used quite a lot in the mocks, and presumably would be also for a user writing a mock?
There was a problem hiding this comment.
I added a DefaultMiddlewareResult interface that represents MiddlewareResult<any, any, any, any>
|
this is awesome 👍 took some wrangling in the end but worth it. |
Type: feature
The following has been addressed in the PR:
prettieras per the readme code style guidelinesDescription:
Provides extended typing of children for function-based widgets, supported by all widget authoring patterns. When children types are defined it makes them mandatory for widget usage.
Typed children provide a first class replacement for widgets that use render props, for example
Outlet.Resolves #528
Resolves #43