Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Flow] Can't "import { type FormProps } from 'redux-form';" #3823

Closed
ryami333 opened this issue Feb 7, 2018 · 12 comments · Fixed by #4352
Closed

[Flow] Can't "import { type FormProps } from 'redux-form';" #3823

ryami333 opened this issue Feb 7, 2018 · 12 comments · Fixed by #4352

Comments

@ryami333
Copy link
Contributor

ryami333 commented Feb 7, 2018

This appears to be because Form.js.flow is not published. In index.js.flow there are these relevant lines:

import type { Props as _FormProps } from './Form'
...
export type FormProps = _FormProps

However ./Form will not resolve to a .flow file, because one does not exist in node_modules/redux_form/es or node_modules/redux-form/lib.

This means that these docs are actually all incorrect: https://redux-form.com/7.3.0/docs/flow.md/. You can however workaround this by using this instead:

import { type FormProps } from 'redux-form/es/types.js.flow';
@ryami333
Copy link
Contributor Author

@erikras this is becoming a bit of a blocker on a project I'm currently working on - would be happy to submit a PR but would be nice to hear your thoughts on this first because it seems to be a fundamental issue with the way types are exported/published in this library, and I could make a quick-fix for this isolated case but I wouldn't be confident that there wouldn't be any regression.

Also, there's already an open PR for a simple Flow fix, and it's sat there unresolved now for about 3 weeks and I'd need some kind of indication that you'd be able to review/merge/deploy a bit more promptly than that.

@ryami333
Copy link
Contributor Author

ryami333 commented Mar 6, 2018

@erikras - final offer! Would you like help safely publishing flow types or no??

@gustavohenke
Copy link
Collaborator

Hi @ryami333, please submit us the PR if you can find the fix for it.
Then we can publish a patch version with it.

@ryami333
Copy link
Contributor Author

It's not a 'fix' so much as a total overhaul is required to the way types are maintained and published in this package. This is why I'd like some engagement before I begin work on it, because it's non-trivial to implement. I've used flow-copy-source with success in a couple of other open-source libraries now - how would you feel about using this tool here too?

@ecnaidar
Copy link
Contributor

ecnaidar commented Mar 12, 2018

@ryami333 also worth noting that FormProps that this package (probably) wants to export is not the one that is exported from Form.js but the one that is defined in types.js.flow

so currently
import type { FormProps } from "redux-form"; is not working for me
import type { FormProps } from "redux-form/lib/types"; gives proper typings for form

https://github.com/erikras/redux-form/blob/master/src/types.js.flow#L93

@JalilArfaoui
Copy link

This issue should be renamed "Broken Flow types import" or something, because it's not specific to FormProps 😢

@JalilArfaoui
Copy link

For FieldProps, the correct import is:

import { type FieldProps } from 'redux-form/es/FieldProps.types.js.flow'

asazernik added a commit to asazernik/redux-form that referenced this issue Jan 12, 2019
Documentation states that 'FormProps' is what's passed *to* a component
wrapped with reduxForm(); however, what was exported here was the props
that should be passed *by* a user *to* the <Form> component.

Switched to using the same naming convention as is already used for
other prop-types with the same potential for confusion: exported
'FieldProps' for the props Field passes to user components, and
non-exported FieldInputProps for the props Field expects to get from its
callers.

Fixes redux-form#3823.
@asazernik
Copy link
Contributor

@JalilArfaoui Importing FieldProps works for me; FormProps does not, and the reason it doesn't is not applicable to FieldProps. I suspect there's something specific to your setup. What's the exact way in which it doesn't work? You're getting an incorrect type definition, or Flow is ignoring the type altogether?

asazernik added a commit to asazernik/redux-form that referenced this issue Feb 8, 2019
Documentation states that 'FormProps' is what's passed *to* a component
wrapped with reduxForm(); however, what was exported here was the props
that should be passed *by* a user *to* the <Form> component.

Switched to using the same naming convention as is already used for
other prop-types with the same potential for confusion: exported
'FieldProps' for the props Field passes to user components, and
non-exported FieldInputProps for the props Field expects to get from its
callers.

Fixes redux-form#3823.
@mike1808
Copy link
Contributor

mike1808 commented Mar 23, 2019

I think the author is planning to move to TypeScript because Flow types in this project just don't work very well. For example, if you wrap your component with any HOC (reduxForm, valueMapper) then all your component props are ignored which makes the whole point of typing useless.

I have to write my own types for HOC and other components. I want to join @ryami333 point of understanding whether this project wants to continue using Flow or it is better just to export them to flow-typed and maintain them there by the community.

@ecnaidar
Copy link
Contributor

ecnaidar commented Mar 25, 2019

@mike1808 TBH I think author is not planning to do anything active with this package, besides merging pull requests.
There is recent form project that seems to be more active
https://github.com/final-form/final-form
So I think the flow updates would be merged, but might take a while. Having them in flow-typed repo might enable people to have quicker updates and necessary fixes.

erikras pushed a commit that referenced this issue Apr 10, 2019
* index.js.flow: Use re-export syntax instead of import/export pair

Minor convenience to the reader (no underscores to distinguish) and
writer (less characters).

* index.js.flow: Export FormProps we pass *to* components

Documentation states that 'FormProps' is what's passed *to* a component
wrapped with reduxForm(); however, what was exported here was the props
that should be passed *by* a user *to* the <Form> component.

Switched to using the same naming convention as is already used for
other prop-types with the same potential for confusion: exported
'FieldProps' for the props Field passes to user components, and
non-exported FieldInputProps for the props Field expects to get from its
callers.

Fixes #3823.
@erikras
Copy link
Member

erikras commented Apr 11, 2019

Published fix in v8.2.0.

@lock
Copy link

lock bot commented Apr 15, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Apr 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants