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

Some TypeScript definitions #58

Closed
wants to merge 3 commits into from

Conversation

sykesd
Copy link

@sykesd sykesd commented May 19, 2017

This is an initial version of a TypeScript typings file - see #8

This version is enough to get a very simple example to compile. However, the file is incomplete, and the types of some things are still unclear to me. If you are interested in me finishing this up to a point where you could accept it, let me know.

If you are, I will definitely have some questions around what types certain things should have.

@jeffbski
Copy link
Owner

@sykesd awesome! I'll take a look at what you have started. I definitely would like to get this setup. I just haven't been using TypeScript yet so having someone with experience is a definite plus.

@sykesd
Copy link
Author

sykesd commented May 23, 2017

Great. I have left TODO comments in the places where I know the types are incomplete. There are bound to be other cases where the types are incorrect/incomplete that I have missed.
Let me once you have had a chance to take a look.

@jeffbski
Copy link
Owner

jeffbski commented Jun 3, 2017

Nice work @sykesd, this is shaping up nicely.

@malafreniere
Copy link

Any ETA on this ? :)

@sykesd
Copy link
Author

sykesd commented Aug 21, 2017

Not at this time. I will put together a list of specific questions I have regarding the parts that are missing and forward them to you as soon as I can.

@Verbon
Copy link

Verbon commented May 8, 2018

Any updates on this? Are TypeScript definitions gonna be released?

@jeffbski
Copy link
Owner

jeffbski commented May 8, 2018

@Verbon I'll see what I can do. I am new to typescript but will see if I can continue the work that @sykesd started.

@sykesd
Copy link
Author

sykesd commented May 9, 2018

Really sorry for the radio silence on this. We are using redux-logic at work and I got pulled away onto other projects for quite some time. But I am back on a project making use of redux-logic.

The types, as present in this merge request still work for us. However, to get to a point where these type definitions can be included in the project itself there are several holes that need to be filled. These are marked with TODO in the file itself.

But I will repeat the main ones here.

Fundamental Questions

  1. Should the Redux Action type be typed as an FSA (Flux Standard Action)?
    We use this quite a bit in our code to add type information, but I realise that not everybody else might.

  2. I have a feeling that some of the parameterized types are incorrectly defined.
    Specifically, I am afraid that I have done some of the "Don'ts" in this list.

More Specific Questions

  1. There are several places in redux-logic where an rxjs observable is exposed. I am not familiar with rxjs at all, and we don't use it explicitly, so I have not invested the time to learn enough to describe those types.
    Can anybody shed some light on what the type of this observable might be?
    An example of such a case is here:

    interface LogicHookParams {
      getState: () => any;
    
      action: LogicAction;
    
      ctx: object;
    
      // TODO cancelled$ observable - no idea yet on what its type signature should be
    }
  2. I define a type ActionTypeMatcher for the type property of a Logic. The docs state that this can be a regular expression, but I am unsure if this is a JavaScript RegExp or just a string containing a regular expression?

There are a couple of other questions, but these are probably a good place to start.

@alvis
Copy link
Contributor

alvis commented Jun 19, 2018

Sorry to cross the post. I was working on a project with redux-logic few months back. At that point this thread was a bit silent so I decided to make my own definition, which is available at https://github.com/alvis/redux-logic/blob/typescript/index.d.ts

It's still WIP, and I also share the same question about FSA as @sykesd. However, it's almost complete as it works fine with my project and it also come with a type test, which includes example usages. There's a few holes from full coverage as I don't use these features, but I'm happy to complete them. Mind me opening another PR for review?

@jeffbski
Copy link
Owner

@alvis Sounds great. I would love to get that added.

@sykesd
Copy link
Author

sykesd commented Jun 26, 2018

@alvis Looks really good!

@jeffbski I think @alvis PR is more correct that mine, not only because he also has types for the rxjs properties, but because of the way he has declared the types from your library itself. I believe it avoids many of the concerns I had about doing it the "wrong" way.

@alvis alvis mentioned this pull request Jul 3, 2018
@jeffbski
Copy link
Owner

jeffbski commented Aug 6, 2018

Thanks for your efforts @sykesd. I have merged @alvis PR and published as 0.15.3

@jeffbski jeffbski closed this Aug 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants