Skip to content

Conversation

@jcperez-ch
Copy link
Contributor

@jcperez-ch jcperez-ch commented May 8, 2017

Small work here for typescripts fans:

  • ignoreInitialState?: boolean;
  • rename Options to UndoableOptions.
  • export UndoableOptions for type checking.

No less, no more.

*Update
FilterFunction also needed some updated params.

@jcperez-ch
Copy link
Contributor Author

Does anybody concerns about typescript users?

@davidroeca
Copy link
Collaborator

@jcperez-ch would it make sense to add a simple typecheck as a prepublish step? I use flow, which has optional static types and has posed no problems here. I haven't used typescript, but a simple check telling me that I haven't provided appropriate annotations might be nice

@jcperez-ch
Copy link
Contributor Author

jcperez-ch commented May 10, 2017

@davidroeca It would make sense just in the case the source code is written with type annotations. Since we are not, we need to generate the declaration file manually, which is fine for now, it was just missing some important types.

If the decision is to start typing the source code (either flow or typescript) then we can probably generate the typescript declaration type automagically. For now, I believe typescript users will have issues if they want to use some configs and some params in the filterFunction.

@davidroeca
Copy link
Collaborator

Interesting. If there's no way to run an external type check, I'm thinking that these definitions may be better off in https://github.com/DefinitelyTyped/DefinitelyTyped so they can be better maintained by the typescript community. Let me know your thoughts on this

@jcperez-ch
Copy link
Contributor Author

As a typescript user, I always believe that importing a lib that has it's own type definitions is way cooler.

So the community shouldn't worry about importing from DefinitelyTyped, since types aare already defined.

Also, if we/you decide to make redux-undo source code in typescript, we would already have the type definitions. Also, the typings.d.ts is already pretty solid it just needed a bit of love.

@davidroeca
Copy link
Collaborator

Makes sense to me. Transpilation from typescript files may prove challenging for compatibility purposes though I'd have to learn more about it. For the time being, typescript users should make sure to keep sending pull requests like this one if type definitions are forgotten/wrong in other PRs.

@omnidan I think this one is likely ready to be merged as well -- if you merge this one first, I'll fix the merge conflicts in #161

@omnidan
Copy link
Owner

omnidan commented May 13, 2017

@davidroeca this PR looks good to me 👌 I'll merge this, please resolve the merge conflicts in #161. When #161 gets merged, we can also think about releasing a new beta version.

@omnidan omnidan merged commit dc1a416 into omnidan:master May 13, 2017
@jcperez-ch jcperez-ch deleted the typings-options branch June 2, 2017 14:45
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.

3 participants