Skip to content
This repository has been archived by the owner on Sep 10, 2022. It is now read-only.

FlowTyped HOCs across files #671

Closed
kirkness opened this issue May 16, 2018 · 27 comments
Closed

FlowTyped HOCs across files #671

kirkness opened this issue May 16, 2018 · 27 comments

Comments

@kirkness
Copy link

I'm essentially trying to create a file with a reusable HOC called withNavbar, it is just a setStatic with a few defaults.

This may be a misunderstanding of mine, but I'm seeing the following and can't understand why;

./with-navbar.js

// @flow
import { setStatic } from 'recompose';

const withNavbar = (title: string) =>
  setStatic('navigationOptions', {
    headerTitle: title,
  });

export default withNavbar;

./screen.js

// @flow
import React from 'react';
import { compose, withStateHandlers, type HOC } from 'recompose';
import { Text } from '../styles/components';
import withNavbar from './with-navbar';

const Test = props => <Text>{props.num}</Text>;

const enhance: HOC<*, {}> = compose(
  withNavbar('MyCounter'),
  withStateHandlers({ num: 1 }, { onSetNum: () => (num: number) => ({ num }) })
);

export default enhance(Test);

When I remove withNavbar('MyCounter') OR move the code in ./with-navbar.js to the top of the screen.js file, I get all the inferred props. However with it, props are empty.

@istarkov
Copy link
Contributor

You need to type withNavbar too

@kirkness
Copy link
Author

Hey, @istarkov thanks for helping - could you elaborate? What is the reason that it works when the hoc is defined within screen.js and not when it's in a different file?

I've tried typing it but am still not seeing it pull through.

@istarkov
Copy link
Contributor

It's how flow works, if something is used inside one file and not exported flow can infer function types.
But if function is exported, flow have no enough information how this function can be used (it's like deprecated * type which infered only within current file)
As I understand this is done by flow team for performance reasons.

In your case withNavbar have same return type as setStatic HOC<A, A> so you could define it as

/* @flow */

import { setStatic } from 'recompose';
import type { HOC } from 'recompose';

const withNavbar = <T>(title: string): HOC<T, T> =>
  setStatic('navigationOptions', {
    headerTitle: title,
  });

export default withNavbar;

@istarkov
Copy link
Contributor

Also having that * is deprecated I would recommend to type Enhanced components as

const SomeComponentExt: React.ComponentType<EnhancedProps> = compose(
   hoc1, ..., hocN
)(SomeComponent)

export default SomeComponentExt;

@istarkov
Copy link
Contributor

The best of above you don't need any type imports from recompose like import type {HOC}
for usual components

@istarkov
Copy link
Contributor

As I understand this is done by flow team for performance reasons.

Also exported function can be used outside of your project so not only performance

@kirkness
Copy link
Author

This is awesome thanks so much for clearing it up!

@FezVrasta
Copy link

FezVrasta commented May 24, 2018

How can I use setStatic without HOC?

const enhance = setStatic('foo', 'bar');

const Foo: ComponentType<{ children: Node}> = enhance(({ children }) => children);

console.log(Foo.foo); // Flow error,  property foo is missing in React.StatelessFunctionalComponent [1].

@istarkov
Copy link
Contributor

@FezVrasta better to use resources like stack overflow as it already have answers on this, as it's the same how to type any react component with static fields
const Foo: { x: string } & React.ComponentType<{ blabla }> = Radio;

@FezVrasta
Copy link

Doesn't this nullifies all the automatic inference that should happen with the Recompose support for Flow? This stuff should be automatically inferred

@istarkov
Copy link
Contributor

What exactly nulified in your case?

@FezVrasta
Copy link

That I have to manually specify the type of my final enhanced component. Other HOCs will automatically add the needed inferred types.

@istarkov
Copy link
Contributor

What changed? You need to specify that type before in HOC, you need to specify it now.

@istarkov
Copy link
Contributor

You need to specify type of your classes so it's all the same

@FezVrasta
Copy link

FezVrasta commented May 24, 2018

I don't know, it seems redundant.

Anyway your suggestion doesn't work, Flow complains:

Cannot assign `enhance(...)` to `Foo` because property `foo` is missing in `React.StatelessFunctionalComponent` [1] but exists in object type [2].

@istarkov
Copy link
Contributor

Because setStatic type is wrong https://github.com/acdlite/recompose/blob/master/src/packages/recompose/index.js.flow#L174

must be something like

declare export function setStatic<A, KEY, VAL>(key: Key, value: VAL): ((React.ComponentType<A>) => {[KEY]: VAL} & React.ComponentType<A>) but need to be tested

@FezVrasta
Copy link

Thanks. I fixed the typo (Key => KEY) but it still doesn't work :-(

@istarkov
Copy link
Contributor

istarkov commented May 24, 2018

flow is magic ;-)
To be clear: in no case I'm big specialist of flowtype, most of time it works well for me, but I also experiencing problems with it, strange errors, and all that magic. Usually if I see errors like above I use "poke method" (метод тыка) so I try different hacks which worked for me in the past, I read flow issues, stack overflow, and do same stuff as other people.
I will look later. The problem of "poke" method that it doesn't give any guaranty that I'll find something.

@istarkov
Copy link
Contributor

For a fast fix you can cast to any and then to your type
const Blabla: MyType = (anything: any);

@FezVrasta
Copy link

Thanks, it's really discomforting to see that one of the most popular React libraries, maintained by one of the Facebook guys, has so bad Flow support, technology built at Facebook 😥

@istarkov
Copy link
Contributor

Current maintaineers are not facebook employes. Mostly I wrote flow support, and having that I never used setStatic it has problems. Also & types were always problematic in flow, and in most cases broke all. Most of hocs works well with flowtype as I use flowtype and recompose on daily basis.
I dont think that flow support bad in recompose

@FezVrasta
Copy link

I see, I'd love to be good enough in Flow to contribute to it.

Maybe moving the types into the library may help to get more contributions? Setting up flow-typed is pretty complex and time intensive.

@istarkov
Copy link
Contributor

Types are already in library see
here is type definitions https://github.com/acdlite/recompose/blob/master/src/packages/recompose/index.js.flow#L174

here is type tests
https://github.com/acdlite/recompose/tree/master/src/packages/recompose/__tests__/types

library itself is not typed, but at the moment I wrote typedefs it wasn't possible because of some flow issues.

@istarkov
Copy link
Contributor

running in project root

yarn flow

you get the all flowtype tests, editor support vscode, nuclide atom is also work well in any file marked with /* @flow */ in project

@istarkov istarkov mentioned this issue May 24, 2018
@istarkov
Copy link
Contributor

istarkov commented May 24, 2018

@FezVrasta see #678
So type I proposed, works, just can't be used inside compose.
And even I'm not a big specialist in flowtype I can give 99% guaranty that it is impossible with current flowtype to make compose work with setStatic inside (without breaking inference for all the hocs).

@FezVrasta
Copy link

Ok thanks for all the info and for your time!

@istarkov
Copy link
Contributor

Also to be fair, last months I use render props components more and more like https://github.com/renatorib/react-powerplug and flow was one of the most thing which affects my move into render props world. Much more easier to use such components with flow, in a lot of cases you dont need to type anything. So can highly recommend at least library above

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants