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

4.0.1 typescript error TS2322 on subscriptions #65

Closed
rosskevin opened this issue Jan 17, 2018 · 12 comments
Closed

4.0.1 typescript error TS2322 on subscriptions #65

rosskevin opened this issue Jan 17, 2018 · 12 comments

Comments

@rosskevin
Copy link
Contributor

rosskevin commented Jan 17, 2018

Are you submitting a bug report or a feature request?

Bug

What is the current behavior?

I introduced changes in #61 that produce typescript errors.

What is the expected behavior?

One example:

error TS2322: Type '{ subscription: { errors: true; values: true; }; children: (form: FormSpyRenderProps) => Element; }' is not assignable to type 'IntrinsicAttributes & FormSpyProps & { children?: ReactNode; }'.
  Type '{ subscription: { errors: true; values: true; }; children: (form: FormSpyRenderProps) => Element; }' is not assignable to type 'FormSpyProps'.
    Types of property 'subscription' are incompatible.
      Type '{ errors: true; values: true; }' is not assignable to type 'FormSubscription'.
        Property 'active' is missing in type '{ errors: true; values: true; }'.

What's your environment?

final-form ^4.0.1
react-final-form ^3.0.2

Other information

I created the problem, I'll PR the fix ASAP. Working on it now. I will probably PR tests to react-final-form as well.

@erikras
Copy link
Member

erikras commented Jan 17, 2018

Thanks for submitting this issue to alert others that it's being worked on. 👍

@rosskevin
Copy link
Contributor Author

rosskevin commented Jan 17, 2018

@erikras - notice that an indexer is added to both the flow and ts types for Subscription. I don't think that any field (key) is allowed here. Should it be 100% open or only the known keys?

@erikras
Copy link
Member

erikras commented Jan 17, 2018

Not sure what you mean by indexer. Point me to a line. You mean this?

@rosskevin
Copy link
Contributor Author

rosskevin commented Jan 17, 2018

Yes exactly, that means it allows any field (key) with a boolean value, which I think is incorrect.

@erikras
Copy link
Member

erikras commented Jan 17, 2018

Aaahh.. It should only be known keys. So really the whole Subscription type can go away, I guess. It's not providing any value.

@rosskevin
Copy link
Contributor Author

It is now, I'll adjust flow like ts. Subscription will be common fields.

@erikras
Copy link
Member

erikras commented Jan 17, 2018

But why not just have FormSubscription and FieldSubscription? There's nowhere where we need an object that could be either.

@rosskevin
Copy link
Contributor Author

FinalForm.js uses the generic Subscription and switching to flow FormSubscription is causing errors. Haven't had a chance to look at it yet. Will do shortly.

@rosskevin
Copy link
Contributor Author

@erikras I bailed out on the flow type changes. It started down a rabbit hole that I'm not quite prepared to follow. I fixed the ts types so we need to mirror them to flow and deal with the subsequent fallout in a different issue/PR.

@erikras
Copy link
Member

erikras commented Jan 17, 2018

I bailed out on the flow type changes

I'm okay with that. 👍

@erikras
Copy link
Member

erikras commented Jan 17, 2018

Published fix in v4.0.2.

@lock
Copy link

lock bot commented Jan 17, 2019

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 Jan 17, 2019
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

2 participants