Flow typings #241
Flow typings #241
Conversation
|
!!!! This is great! I will review this ASAP |
|
@acdlite these are some "manual" tests I wrote while developing the libdef. They are not meant to be exhaustive (I'm sure I missed something), though they can be helpful in order to quickly try out the libdef import React from 'react'
import ReactDOM from 'react-dom'
import type { Component } from 'recompose'
import {
compose,
mapProps,
withProps,
withHandlers,
defaultProps,
renameProp,
renameProps,
flattenProp,
withState
} from 'recompose'
const app = document.getElementById('app')
type Props = {
a: string,
b: number
};
const C1: Component<Props> = (props: Props) => <div><pre>{JSON.stringify(props, null, 2)}</pre></div>
// mapProps
const C2 = mapProps(({ a }: $Shape<{ a: string }>): Props => ({ a, b: a.length }))(C1)
ReactDOM.render(<C2 a="s" />, app) // <= <C2 a="s" xxx="s" /> raises an error
// withProps
const C3 = withProps({ a: 's', b: 1 })(C1)
ReactDOM.render(<C3 />, app)
// withHandlers
const C5_1 = (props: { onChange: Function }) => <div><pre>{JSON.stringify(props, null, 2)}</pre></div>
const C5 = withHandlers({
onChange: props => event => {
props.updateValue(event.target.value)
}
})(C5_1)
ReactDOM.render(<C5 />, app)
// defaultProps
const C6 = defaultProps({ a: 's' })(C1)
ReactDOM.render(<C6 b={1} />, app)
// renameProp
const C7: Component<{cc: number}> = renameProp('a', 'cc')(C1)
ReactDOM.render(<C7 cc={1} />, app)
// renameProps
const C8: Component<{ccc: number}> = renameProps({'a': 'ccc'})(C1)
ReactDOM.render(<C8 ccc={1} />, app)
// flattenProp
const C9: Component<{}> = compose(
withProps({
object: { a: 'a', b: 'b' },
c: 'c'
}),
flattenProp('object') // <= flattenProp('xxx') raises an error
)(C1)
ReactDOM.render(<C9 />, app)
// withState
const C10_1 = (props: { counter: number }) => <div><pre>{JSON.stringify(props, null, 2)}</pre></div>
const C10 = withState(
'counter',
'setCounter',
0
)(C10_1)
ReactDOM.render(<C10 c="c" />, app) |
|
When using Results: Thank you for your work with this PR! |
|
Oh sorry, I think there is an error in my libdef, could you please try this change? -declare function pure<A>(): HOC<A, A>;
+declare function pure<A>(C: Component<A>): FunctionComponent<A>; |
|
@hannupekka just to be sure, you are calling import React from 'react';
import Logo from './Logo';
<Logo styleName="logo" /> |
|
@gcanti sweet, works as expected! And yes, calling it just like you described. |
|
@hannupekka some observations If you call <Logo className={1} /> // <= wrong type for classNameFlow doesn't complain, right? (But it should) The problem is that, due to the current behavior of Flow, you must explicitly type the exported APIs. So my suggestion is to change the definition of // @flow
import React from 'react';
import { pure } from 'recompose';
import type { Component } from 'recompose';
const logo = require('assets/logo.svg');
type Props = {
className?: string
};
const Logo: Component<Props> = (props) => {
return (
<div className={props.className}>
<img src={logo} />
</div>
);
};
export default pure(Logo);It's a little more verbose but it makes sure you get the best type safety. <Logo className={1} /> // <= error: number. This type is incompatible with stringNow if you want to go further and you are using Flow v0.32+ you can even prevent additional props, just change the type Props = $Exact<{
className?: string
}>;and you get <Logo unknown="prop" /> // <= error: property `unknown`. Property not found in object type With the latest babel / babel-eslint you can even use a new syntax (equivalent to the type Props = {|
className?: string
|}; |
|
True, nice catch! After that ESLint might give warnings depending on the config and plugins you are using:
To fix:
|
|
Thank you @gcanti! I've been trying the declaration file. When using type Props = {
a: string,
b: number,
};
const C1: Component<Props> = (props) => <div />
const C3 = withProps(() => ({ a: '1', b: 2 }))(C1)It throws an error: I am not familiar with const C3 = withProps((): Props => ({ a: '1', b: 2 }))(C1) // still error |
|
@wuct Flow seems to not account for the second member of the union declare function withProps<A, B>(
createProps: A | (ownerProps: B) => A
): HOC<A, B>;try this instead (explicit overloading) declare function withProps<A, B>(
createProps: (ownerProps: B) => A
): HOC<A, B>;
declare function withProps<A>(
createProps: A
): HOC<A, {}>; |
|
@gcanti Thanks! But now it works like If I remove a prop returned by type Props = {
a: string,
b: number,
};
const enhance =
withProps(() => ({ a: '1' })) // remove b
const C1: Component<Props> = (props) => <div />
const C3 = enhance(C1)Flow throws an error: I think we can use declare function withProps<A, B>(
createProps: (ownerProps: B) => $Shape<A>
): HOC<A, B>;However, flow become undecided: Am I using |
Ah, you are right. This seems to work as expected declare function withProps<A, B>(
createProps: (ownerProps: B) => A
): HOC<A & B, B>;
declare function withProps<A, B>(
createProps: A
): HOC<A & B, B>;Here my tests const C3_1 = withProps({ a: 's', b: 1 })(C1)
// ok
;<C3_1 />
const C3_2 = withProps({ b: 1 })(C1)
// ok
;<C3_2 a="s" />
// ok
;<C3_2 a="s" b={2} />
// $ExpectError missing property a
;<C3_2 />
const C3_3 = withProps(() => ({ a: 's', b: 1 }))(C1)
// ok
;<C3_3 />
const C3_4 = withProps(() => ({ b: 1 }))(C1)
// ok
;<C3_4 a="s" />
// ok
;<C3_4 a="s" b={2} />
// $ExpectError missing property a
;<C3_4 /> |
|
@gcanti It works! I haven't come up with the intersection type. Thanks a lot! |
|
@wuct thanks for testing out. PR updated |
|
It turns out that we shall also use the intersection type in declare function withHandlers<B, A: { [key: string]: (props: B) => Function }>(
handlerCreators: A
): HOC<A & B, B> |
|
Thanks @wuct. PR updated. |
|
It's awesome!!! Wanna ask how we could change the enhancers like Just played a little with dirty idea like declare function withState2<B, T, A: { [key: string]: (props: B) => T }>(
states: A
): HOC<{ [key: $Keys<A>]: { value: T, setValue: (a: T) => void } } & B, B>usage example const C1 = ({ foo: { value, setValue } }) => (
<div onClick={() => setValue('world')}>
{value}
</div>
);
const C2 = withState2({
foo: ({ fooInitial }) => fooInitial,
})(C1)
<C fooInitial="hello" />It works nice with flow having that all state initializers inside |
|
@istarkov do you reckon this definition as it stands is fairly useful/friendly to the current recompose implementation? i.e. could I manually add them to my project and get by okay, or do you reckon they need more work still? |
|
I've played with this locally and I really like this definitions, BTW I does not use flowtype in my everyday work so can't be an expert. IMO this must be publushed somehow, may be in flowtyped repo, or here, but I really have zero knowledge how to do it, and does just merging this and publishing is enough. Also some documentation and tests are also needed here. If anyone can help with please welcome. |
|
Personally I feel that the official @gcanti - are you happy if we move this on to the I'm happy to help out here as I am adopting recompose for a large scale project. |
Sure 👍 |
|
Has the discussion continued on the |
@ctrlplusb please, feel free to take this PR as a starting point and go ahead, you are in a better position than me ;) |
|
After recompose@0.21.2, declare function withHandlers<B, A: { [key: string]: (props: B) => Function }>(
handlerCreators: (ownerProps: B) => A
): HOC<A & B, B>
declare function withHandlers<B, A: { [key: string]: (props: B) => Function }>(
handlerCreators: A
): HOC<A & B, B> |
|
I've played with current definitions for typescript for recompose, and wanna say that current is much more nicer. Returning to withValue<A, B, T>(
initialState: T | (props: Object) => T ,
mapStateProps: (state: T, setState: (T) => void | etc.. ) => A
): HOC<A & B, B>;Haven't played with but IMO it will allow to have typed definition of Usage example: // old
withState('value', 'onChange', 1)
// new
withValue(1, (value, onChange) => ({ value, onChange })); |
|
@istarkov by current, do you mean Flowtype definition? I haven't played with Typescript so I don't know what is the difference between them :) I like the new version of |
|
Yes flowtype definition. |
|
I'm following @gcanti 's example above and I'm unable to get it to work for the // button.js
const Button: Component<Props> = (props) => (
<Button
...
/>
);
export default compose(
onlyUpdateForKeys(['following']),
)(Button);
// view.js
<Button badProp={badValue} /> // Flow doesn't catch itInterestingly enough, if // button.js
const Button: Component<Props> = (props) => (
<Button
...
/>
);
export default onlyUpdateForKeys(['following'])(Button);
// view.js
<Button badProp={badValue} /> // Flow catches it... then it gets properly flagged. Am I doing something wrong, or does this libdef not support chaining HOCs together via compose? |
|
Is it possible to manually specify the flow type of the enhanced component? I'm not sure what the type signature would look like for this: // button.js
const Button: Component<Props> = (props) => (
<Button
...
/>
);
const enhance: /* ??? */ = compose(
onlyUpdateForKeys(['following']),
)(Button);
export default enhance;I have tried: const enhance: HOC<Button, Button> = compose(
onlyUpdateForKeys(['following']),
);
export default enhance(Button);but no luck there. |
|
What's the status of this (or flow-typed/flow-typed#624)? Is this going to happen any time soon? |
|
Seems like having modern flowtype some HOCs are better to write using Object spread declare function withProps<BaseAdd, Enhanced>(
propsMapper: (ownerProps: Enhanced) => BaseAdd
): HOC<{ ...$Exact<Enhanced>, ...BaseAdd }, Enhanced>;this allows to change property types etc Also seems like I have a few free days, and want to spend them to add typings to recompose. Can someone recommend how to better expose |
|
As I see modern libs use flowtyped https://www.styled-components.com/docs/api#flow |
|
@istarkov flow-typed sucks in siemens. It uses git requests. For npm we have artifactory. |
|
@istarkov |
This PR is a proof of concept and is not meant to be merged.
There are not many examples of HOC typings with Flow out there, so I thought to write this POC.
I hope is helpful as a starting point for an official recompose libdef.
Feedback from recompose + flow users would be much appreciated.