Add TypeScript types #30
Conversation
| interface Options { | ||
| debounce?: boolean, | ||
| events?: Record<string, Array<string>> | ||
| modelTypes?: Record<string, object>, |
There was a problem hiding this comment.
This could probably be more specific than string, to limit it to keyof Models. Up to you if you think that's worth it though.
| withRef?: boolean | ||
| } | ||
|
|
||
| export declare function connectBackboneToReact<Model extends {}, ModelProps extends {}>( |
There was a problem hiding this comment.
Naming nit: shouldn't this be Models, not Model? It's an object with multiple models in it, right?
| } | ||
|
|
||
| export declare function connectBackboneToReact<Model extends {}, ModelProps extends {}>( | ||
| mapModelToProps: (models: Model, props: ModelProps) => Partial<ModelProps>, options?: Options |
There was a problem hiding this comment.
Another naming nit: it should be mapModelsToProps
| export declare function connectBackboneToReact<Model extends {}, ModelProps extends {}>( | ||
| mapModelToProps: (models: Model, props: ModelProps) => Partial<ModelProps>, options?: Options | ||
| ): <CombinedProps>( | ||
| wrappedComponent: React.ComponentType<CombinedProps> | ||
| ) => React.ComponentType<Omit<CombinedProps, keyof ModelProps>>; |
There was a problem hiding this comment.
I think ModelProps might be inconsistent here. As the second param to mapModelsToProps, it should me all of the props passed in from the parent component. But then in the return type you're omitting those props from what needs to be passed in from the parent component.
Compared to what I had written in the JIRA ticket:
function connectBackboneToReact<Models extends {}, ModelProps extends {}>(
mapModelsToProps: (models: Models) => ModelProps
): <CombinedProps>(
WrappedComponent: React.ComponentType<CombinedProps>
) => React.ComponentType<Omit<CombinedProps, keyof ModelProps>>;In that definition, ModelProps was the props specifically being added by mapModelsToProps, so omitting them from the generated component type meant that the parent wouldn't have to pass in anything that was derived from a backbone model. I didn't have a type included for the props param though, so we probably can't just use that as-is. I'm not 100% sure what the best way to write that is where TS can still infer everything properly, tbh.
No description provided.