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

[V6] Anyone created a typescript definition file for v6 yet ? #1211

Closed
rluiten opened this issue Jun 24, 2016 · 54 comments
Closed

[V6] Anyone created a typescript definition file for v6 yet ? #1211

rluiten opened this issue Jun 24, 2016 · 54 comments

Comments

@rluiten
Copy link
Contributor

rluiten commented Jun 24, 2016

Just read through the state and I think v6 shows lots of promise over starting this project with v5 at moment.

I can start with a super basic one and fill out bits as I go but I have little experience yet with creating definitions and I'd probably leave things loser than they could be.

@ooflorent
Copy link
Contributor

Not to my knowledge. Maybe @vslinko has created one .

@vslinko
Copy link
Contributor

vslinko commented Jun 25, 2016

I just started but I switched to v5 for some reasons. So I have no typings for v6.

@ooflorent
Copy link
Contributor

@vslinko Could elaborate more on why v6 doesn't fit with your needs (here, or a separate issue, or a mail)?

@rluiten I'll keep this open to see if someone has previously created the typings but there are no official definitions.

@vslinko
Copy link
Contributor

vslinko commented Jun 25, 2016

@ooflorent It's because of two reasons:

  1. v5 already have TypeScript typings.
  2. I already have my own small toolkit around v5.

@rluiten
Copy link
Contributor Author

rluiten commented Jun 28, 2016

I have created a starting type definition I am using now.
I put it on a public gist.

Disclaimer 1. I am new to creating type definitions.
Disclaimer 2. I am still new to redux-form.

It may contain errors and not be as strict as it could be.

But figured i put it here in case it helps someone else.
If you have improvements I will be happy to incorporate them.

https://gist.github.com/rluiten/a41fb5845fae11a5484bad954d97b3a8

@CarsonF
Copy link

CarsonF commented Jun 28, 2016

I'm finishing up one. I'll gist it so yall can get any idea. I can PR it in as well if we want to keep it here, which I think would be great.

https://gist.github.com/CarsonF/5b9d0d9d8c37c0da5a17bf4ddccd2887

It uses Redux's built-in definition so it's a bit different from @rluiten's

@rluiten
Copy link
Contributor Author

rluiten commented Jun 28, 2016

Well that's a big 👍 from me for @CarsonF
You definitely know much more typescript than me, just skimmed your type def and saw lots of stuff I have no clue about.

Dang just tried, I am using the redux definition form typings which are different to the one in the npm module. I have definitions for other modules that depend on the typings redux definition to. Dang.

I don't believe I can switch away from the redux defintions in typings easilly at moment. :(

@CarsonF
Copy link

CarsonF commented Jun 28, 2016

Yeah...those have been deprecated since redux has them builtin now.

https://github.com/andrew-w-ross/typings-redux

I'm still tweaking it, but overall I think it's pretty close.

@erikras What do you think about having the typescript definition live in this repo?

@rluiten
Copy link
Contributor Author

rluiten commented Jun 28, 2016

I decided I may as well move to off the old redux definition I was using from definitley typed via typings.

Ended up copying about 3 definitions out of typings and tweaking them for use with the the redux node modules definitions that all took a bit of time but wasnt to hard and so far things seem to be all ok.

So then I grabbed your module definition.
I could be doing this wrong so I wont be offended if you call me stupid and correct me :)

@CarsonF I ran into a hiccup it apperas you don't actually create a module.
So the import statements in my typescript code cant find the definition, i think this is a problem but I may not be doing something correctly.

As a test I wrapped your file in a 'declare "redux-form" {}' and removed the declare on the namespace "ReduxForm" and it seems mostly ok. Though some of my code has type errors now I am pretty sure its my fault as my definition was very minimal and loose.

@rluiten
Copy link
Contributor Author

rluiten commented Jun 28, 2016

Ran into another problem, InputFieldProps

does not allow me to pass type="checkbox" into my input field.

I had a look around alpha 15 added a props parameter to to pass through props.
So I added to InputFieldProps

, and switched to using props for the extra parameters.

I don't know if the type can be made tighter or not.

        /**
         * Object with custom props to pass through the `Field` component into a component provided
         * to `component` prop. This props will be merged to props provided by `Field` itself.
         */
        props?: Object; // was this but i was wrong i think {[attributeName:string]: string};

@CarsonF
Copy link

CarsonF commented Jun 28, 2016

@rluiten Thanks for testing this out. Always helps to have a second pair of eyes!

@CarsonF I ran into a hiccup it apperas you don't actually create a module.
So the import statements in my typescript code cant find the definition, i think this is a problem but I may not be doing something correctly.

Yeah it's not an ambient definition. If you look at all the definitions in the npm folder in typings/registry, none of them are ambient. I was installing it with this command:

typings install file:definitions/redux-form.d.ts -S

Ran into another problem, InputFieldProps

does not allow me to pass type="checkbox" into my input field.

Do you mean for the component property? That has to be a component name. You could do this:

<Field component="input" type="checkbox" />

I had a look around alpha 15 added a props parameter to to pass through props.

I remember that now. I'll need to look into that. I would rather declare that type through a generic, that way we get code completion and validation.

@rluiten
Copy link
Contributor Author

rluiten commented Jun 28, 2016

@CarsonF thank you your type system is way tighter than I would likely have gotten to.

Thanks for the clarification.
I see how not using ambient with typings basically wrapped it for me.

Looked at your example in the gist, great idea to add it.
I have not used decorators for any javascript or typescript yet.
In fact I wasn't even aware they were supported in typescript.
Reading up that decorators and what is going on in the example next.

I had not really used static either in typescript, i was just using functions in the vicinity
of my component, not a bad way to group them more specifically.

Learning lots, thanks.

Do you mean for the component property? That has to be a component name. You could do this:

I mean the Field component, that takes component properties yes.
By default Field passes on all extra props to Field to the component specified by the component property. I read through some of the alpha release notes and 15 add the an extra prop called 'props' thanks to a contributor @vslinko who was concerned about types for typescript and I think it was a good idea.

IMO it would be great if we could type limit the arbitrary extra props on Field by the component specified by the 'component' property, I don't see how this is possible but I am new to what can be done with typescript types. A complication factor is that component is a union of 5 types including 3 strings so I think it gets complex. Given we have props maybe its not a bad fall back to have the Field props checked and the 'props' prop as a bucket for pass through though if we can type the 'props' property that would be good.

@CarsonF
Copy link

CarsonF commented Jun 28, 2016

I don't see how adding props property adds any value. There's no way to restrict properties based on one of the property's value (component). And as @erikras said in #1066 all the properties are passed on to the component anyways. So that property doesn't help the typescript definition at all...unless I'm missing something @vslinko?

The type could be declared as a generic on the Field<OtherProps>, but that leads to the next problem. There's no good way to specify generics in JSX (since it uses the same <> syntax). The current code does have generics on Field and FieldArray, which probably needs to change.

I've gotten around both these problems by wrapping the Field component. I needed to do this for Material UI, here's my code:

/**
 * A redux-form Field for Material UI's `TextField` component.
 */
export class TextField extends React.Component<__MaterialUI.TextFieldProps & {ref?: React.Ref<TextField>}, {}> {
  render() {
    return <Field component={MappedTextField} {...this.props} />;
  }
}

/**
 * Maps the `error` property to the `errorText` property.
 */
export class MappedTextField extends React.Component<__MaterialUI.TextFieldProps & FieldProps, {}> {
  render() {
    return <Mui.TextField errorText={this.props.touched && this.props.error ? this.props.error : ''} {...this.props} />;
  }
}

Usage:

<TextField
  id='username'
  name='username'
  floatingLabelText='Username or email'
  onKeyDown={this.handleTextFieldSubmit}
  fullWidth={true}
/>

TextField should have the InputFieldProps interface as well for it's P generic, but that interface requires component. Since we are specifying that property inside this class we don't need to pass it in, so I couldn't declare that. Also Material UI's TextFieldProps interface conveniently has name and defaultValue, so it didn't really hurt me.

This would be something to think about though, maybe component should be made optional.

@rluiten
Copy link
Contributor Author

rluiten commented Jun 29, 2016

@CarsonF I notice your examples are all not Stateless components, is there some reason for that ?
Maybe a type related reason or some other.

As an aside are you using the React type definition from definitely typed v0.14 is latest i believe ?

I have been on this project for a few months with react/redux/typescript and I still feel like i have a lot to learn dealing with typescript.

@CarsonF
Copy link

CarsonF commented Jun 29, 2016

@CarsonF I notice your examples are all not Stateless components, is there some reason for that ?
Maybe a type related reason or some other.

Yeah IntelliJ/WebStorm/etc. doesn't provide completion for the stateless components. I also don't think that this way is that much more verbose.

As an aside are you using the React type definition from definitely typed v0.14 is latest i believe ?

From DT yeah. 15.1.0 now I believe.

I have been on this project for a few months with react/redux/typescript and I still feel like i have a lot to learn dealing with typescript.

Haha same here, this is all new to me too :)

I just update the gist with the props property, no more completion than Object though.

@rluiten
Copy link
Contributor Author

rluiten commented Jun 29, 2016

I am trying out how you set things up but I appear to have hit a hiccup I am stuck on.

import React = require('react');
import { Field, FieldProps } from 'redux-form';

interface MyFieldProps {
    message: string;
    name: string;
}

export class TestField extends React.Component<MyFieldProps, {}> {
    render() {
        return <Field component={MyField} {...this.props} />; // <<< line 11
    }
}

export class MyField extends React.Component<MyFieldProps & FieldProps, any> {
    render() {
        return (
            <div {...this.props} >
                {this.props.message}
                <div>{this.props.touched && this.props.error
                    ? 'Error: ' + this.props.error
                    : ''}</div>
            </div>
        );
    }
}

I get the the following error.

(11,23): error TS2322: Type 'typeof MyField' is not assignable to type 'Component<P, any> | StatelessComponent

| "input" | "select" | "textarea"'.
Type 'typeof MyField' is not assignable to type '"textarea"'.

Which has me at a loss at the moment.
Sorry to keep asking questions you are a veritable font of information... 👍

Extra Info.
If I switch to Stateless Component same problem.
With export function MyField (props: MyFieldProps & FieldProps)

If I switch to export function MyField (props: any)
Error goes away.

Switch to any for Component<> still gives error.

@CarsonF
Copy link

CarsonF commented Jun 29, 2016

Yeahhhhh I was just ran into that too. I had those mapped components as stateless until I commented above. I guess this way provides enough info to fail it haha.

Try changing that type to

component?: typeof Component | typeof StatelessComponent | ...

@rluiten
Copy link
Contributor Author

rluiten commented Jun 29, 2016

This fails.

    component?: typeof Component | typeof StatelessComponent | ...

But
This worked

    component?: typeof Component | StatelessComponent<P> | ...

Spoke to soon, id forgot id set my props to any in my state component.
It does work if the State component has props of any, but if i try to put type there its same as previously.

Hmm it worked in one case not another.

Summary. using

component?: typeof Component | StatelessComponent<P> | ...

I can create classes fine and set actual types for props it works right.
I can't create stateless functions and set actual types for props get error (can use any though).

@rluiten
Copy link
Contributor Author

rluiten commented Jun 29, 2016

This appears to be relevant, coming in typescript 2.0.
microsoft/TypeScript#8674

@rluiten
Copy link
Contributor Author

rluiten commented Jun 29, 2016

Just noticed.
In Config<FormData extends DataShape, P, S>
I think validate needs to be

validate?(values: FormData, props: P): FormErrors<FormData>;

@CarsonF
Copy link

CarsonF commented Jun 29, 2016

@rluiten Updated the gist with a fix for the component prop. I made name and component optional too so that interface could be used with a wrapped component that provides those properties.

Updated version above example:

export class TextField extends React.Component<InputFieldProps<__MaterialUI.TextFieldProps> & __MaterialUI.TextFieldProps & {ref?: React.Ref<TextField>}, {}> {
  render() {
    return <Field component={MappedTextField} {...this.props} />;
  }
}

notice the InputFieldProps<__MaterialUI.TextFieldProps> & for the component's P generic.


I think validate needs to be

validate?(values: FormData, props: P): FormErrors;

I know that's the definition in the docs, but it's incorrect. Maybe it's a bug, either way I didn't find it useful, so I left it out. It's actually the properties of the redux connected form component which wraps the form.
screenshot

Basically something like this:

export interface Config<FormData extends DataShape, P, S> {
  validate?(values: FormData, props: Config<FormData, P, S>): FormErrors<FormData>;
}

@rluiten
Copy link
Contributor Author

rluiten commented Jun 30, 2016

Good point on the type of validate() second parameter.
I see you picked up the component parameter for all cases and extracted to a type nice.

Is there a type for the properties passed to a FieldArray render component ?
The two primary properties appear to be.

fields: which is a FieldArrayProps<T>
name: which is field name

Maybe something like this, i could not find any interfaces with a field fields.

interface FieldArrayRenderProps<T> {
    fields: FieldArrayProps<T>;
    name: string;
}

I am using it this way at the moment

interface RenderServicesProps extends FieldArrayRenderProps<VM.ISupportServicesViewModel> {
}

class RenderServices extends React.Component<RenderServicesProps, {}> { /* stuff */ }

Which is working fine at the moment.

@rluiten
Copy link
Contributor Author

rluiten commented Jun 30, 2016

Just got bitten by this in my checkbox field.

Referring back to example above with TestField just found out I have to specify the type either in the Field properties directly or in the special field property 'props' for inputs that are of type "checkbox" "radio" and "multiple-select" as processProps() in createFieldProps.js in source base use the properties "type" and "value" to adjust properties for contained elements eg. the value of "checked".

I have modified my helper component to.
Now my checkbox fields are properly checked or not based on value being true or false

export class CheckboxField extends React.Component<CheckboxFieldProps, {}> {
    render() {
        return <Field component={CoreCheckboxField} {...this.props} props={{type : "checkbox"}} />;
    }
}

I am re-considering adding type?: string on InputFieldProps<P>.

Just discovered that I can not pass in property value on the props field.
I need to pass it as a direct property to Field or it is not set on the input field, its required for radio inputs to set what value they have on checked, which in turn affects how the checked value is derived by the redux-form processProps(). I added a comment to redux-form pulle request #1078 for this one.

@rluiten
Copy link
Contributor Author

rluiten commented Jul 7, 2016

I updated my gist its mainl CarsonF's version with some updates I have applied since.

https://gist.github.com/rluiten/a41fb5845fae11a5484bad954d97b3a8

@kristian-puccio
Copy link

I know this is the wrong place for this, when I get time I'll make a pull
request to flow-typed but here is my working flow definition:

https://gist.github.com/kristian-puccio/65d32833306ba8b697b1b3db041aeb44

On 8 July 2016 at 08:32, Robin notifications@github.com wrote:

I updated my gist its mainl CarsonF's version with some updates I have
applied since.

https://gist.github.com/rluiten/a41fb5845fae11a5484bad954d97b3a8


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#1211 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ACA_f9FBXQWplRXxmGKRSglz-QahE1xAks5qTX6MgaJpZM4I9cLM
.

@rluiten
Copy link
Contributor Author

rluiten commented Jul 8, 2016

I updated the redux-form.d.ts for 6.0.0-rc3 - which so far works with my code and the split properties for Field.
Updated gist, (same url).
https://gist.github.com/rluiten/a41fb5845fae11a5484bad954d97b3a8

@CarsonF
Copy link

CarsonF commented Jul 8, 2016

@rluiten It's probably not good to have two copies of the definition floating around. If you've just copied mine and modified it a bit, can you just send me that patch instead?

Still haven't heard about whether they will accept a PR for this or not. I guess I'll start with a PR but move it to a separate repo if they don't want it here.

@UweWindt
Copy link

UweWindt commented Jul 8, 2016

where is the 'master' of redux-form.d.ts?
I'm a bit confused when i'm reading the comments...

@CarsonF
Copy link

CarsonF commented Jul 8, 2016

@UweWindt Well currently there are two versions. Both of the gists will have the latest code though. I'm updating mine right now for rc3. I'll PR soon after.

@UweWindt
Copy link

UweWindt commented Jul 8, 2016

what does PR mean?
(Sorry for this silly question)

@CarsonF
Copy link

CarsonF commented Jul 8, 2016

Ok I updated the gist. I renamed FieldProps to WrappedFieldProps for clarity. And renamed InputFieldProps to BaseFieldProps so it isn't confusing with new interface WrappedFieldInputProps which is the new input property (@rluiten called this interface BaseInputProps).

Need to fork and make commits, then I'll PR since @erikras gave me the thumbs up :)

@CarsonF
Copy link

CarsonF commented Jul 8, 2016

PR's up! #1318

@Strato
Copy link
Contributor

Strato commented Sep 14, 2016

If you have a reasonably working definition for v6, can't you just PR it directly to Definitely Typed? It will also make contributions easier.

I think Erik has too much to deal with right now...

@CarsonF
Copy link

CarsonF commented Sep 14, 2016

The same problem could happen there as well.

@fuzing
Copy link

fuzing commented Nov 8, 2016

Folks - in light of proposals to 'fix' typings with TS 2.x (See here) are there any updates on typings for V6? Will they be incorporated directly into the redux-form repo, or into the @types eco-system? Do we know when? Thanks for your great work.

@LKay
Copy link

LKay commented Nov 15, 2016

Based on typing by @CarsonF I rewrote some part, added missing things and here are the (mostly) working typings for redux-form@6.2.0 based entirely on API Docs and personal experience:

https://gist.github.com/LKay/73a2eb2e706c54e856390a69b5993e8a

I could use it pretty fine for couple of my projects and haven't encountered any major problems. Due to TypeScript limitation of extending interface with generics (discussed in Issue #2225 there is still need to explicitly define custom props type for Field, Fields and FieldArray (example in gits).

The redux-form/immutable should also work, but as I don't use ImmutableJS in projects I use forms I couldn't test many cases. Any help appreciated.

Can you, folks test it in your projects and report any problems? I'd like to have it tested quite thoughtfully before adding to DefinitelyTyped.

@Strato
Copy link
Contributor

Strato commented Nov 15, 2016

Thank you LKay. I gave it a quick try. I'm using 'redux-form/immutable'.

Some remarks :

  • I had to add this to the top of the defition file:
/// <reference types="react"/>
/// <reference types="immutable"/>

And this to the Immutable definition file:

export = Immutable;
export as namespace Immutable;

There might be another way, especially without changing the Immutable definition, but I had no better idea and not much time.

  • Last React definition now has event handlers generically typed. Had to fix those:
/* Field, Fields, FieldArray commons */
interface WrappedInputProps {
    checked: boolean
    name: string
    onBlur: React.FocusEventHandler<any>
    onChange: React.FormEventHandler<any>
    onDragStart: React.DragEventHandler<any>
    onDrop: React.DragEventHandler<any>
    onFocus: React.FocusEventHandler<any>
    value: any
}

Couldn't find the right syntax to type them properly (without the any).

  • The reason I won't implement it for the moment is that all my components using ReduxForm are flashing red right now, because they use <Field /> with additional props. I've seen your example showing how to create a safe type, but that's a lot of work to do it everywhere. I'm not sure what would be best to ease the migration. For the moment I have a stub definition that declares <Field /> as any.

@LKay
Copy link

LKay commented Nov 15, 2016

@Strato About the first two. I was using immutable typings from DefinitelyTyped, which do declare proper namespace for Immutable. Maybe you're using a custom ones or some old version but that export as namespace seems unnecessary. About react typings, event handlers are not generics anymore for current definitions for 14.4 and above fro. DefinitelyTyped. And yes, I assume you have this namespaces available in scope, so if you don't have them, you need to ad references.

The <Field /> is tricky part as it passes down the properties, and Typescript does not support generic extending for interfaces at the moment. I could add [prop: string]: any to the props base props interface but that kills the purpose of type safe definition. Might be tedious, but seems like necessary evil if you want to keep your code type safe.

@zakdances
Copy link

There's not even a v5 up yet. Latest definition is v4.0.3.

@CarsonF
Copy link

CarsonF commented Nov 29, 2016

I'm working on getting my definition there and finishing it. However, currently there's a problem with the redux definition. I built my definition of off the new redux one, but DT still has the old one.

DefinitelyTyped/DefinitelyTyped#11551
DefinitelyTyped/DefinitelyTyped#12965

@ianlyons
Copy link

ianlyons commented Dec 2, 2016

@CarsonF looks like the DT PR got merged

@CarsonF
Copy link

CarsonF commented Dec 2, 2016

Yes it did. Working on this tonight.

@sawatsky
Copy link

sawatsky commented Dec 6, 2016

@CarsonF You aren't declaring a module in your ts definition. Can you please tell me how to modify it to declare a module in my copy, or tell me how to use yours without a module? I'm really new to react, redux, and typescript.

@grovesNL
Copy link

grovesNL commented Dec 6, 2016

@nzjoel1234
Copy link

@CarsonF It seems you have missed a few things in the FormProps interface... I have noticed that destroyOnUnmount?: boolean and initialValues?: FormData are both missing. Does this sound correct to you or am I missing something?
Cheers

@CarsonF
Copy link

CarsonF commented Jan 9, 2017

@nzjoel1234 Are those passed into the form component? I don't remember. They are specified on the form config:
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/redux-form/lib/reduxForm.d.ts#L64

@CarsonF
Copy link

CarsonF commented Jan 9, 2017

All: New issues should be opened on DefinitelyTyped since the definition lives there.

@erikras Can you close / lock this?

@nzjoel1234
Copy link

Sorry should have raised it there...
DefinitelyTyped/DefinitelyTyped#13879

@tugberkugurlu
Copy link

Is there any movement on this? I think definitely typed still has v4 typings.

@LKay
Copy link

LKay commented Mar 29, 2017

@tugberkugurlu DefinitelyTyped has typings for version 6.3 for a while now. Also PR to update to version 6.6 is waiting to be merged here.

@danielrob
Copy link
Collaborator

Thanks @CarsonF, closing this issue as TS type definitions are now stored on DefinitelyTyped

@redux-form redux-form locked and limited conversation to collaborators Aug 3, 2017
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