veliovgroup / flow-router

🚦 Carefully extended flow-router for Meteor
https://packosphere.com/ostrio/flow-router-extra
BSD 3-Clause "New" or "Revised" License
201 stars 30 forks source link

[feature request] Are there any typescript type definitions? #84

Closed bessw closed 1 year ago

bessw commented 3 years ago

Are there any typescript type definitions?

how this could be done:

dr-dimitru commented 3 years ago

Hello @bessw ,

No, never crossed paths with TypeScript, so no plans to implement it. You're more than welcome to state a solution proposal in this thread. Then after we all agree on further steps, I'll accept your PR.

Things to note:

afrokick commented 3 years ago

DefinitelyTyped is the right way for declarations. I can help with it.

dr-dimitru commented 3 years ago

@afrokick Thank you for jumping in. I'd happily merge a PR. Is there a way to validate Definitions as a part of auto-tests?

bessw commented 3 years ago

I haven't done much with own type definition files, since I write my code in TypeScript, but my search results are:

dr-dimitru commented 3 years ago

@afrokick @bessw Meteor's forum thread

afrokick commented 3 years ago

Is there a way to validate Definitions as a part of auto-tests? Yes, you can run tsc --noEmit on tests files to check definitions.

I found this one https://github.com/meteor-typings/flow-router We can adapt it.

dr-dimitru commented 3 years ago

@afrokick Same link is in OP ;) I'm all up for it - reuse and adapt.

Would it be compatible with DefinetelyTyped or we are going to stick with TypeSript?

timsun28 commented 2 years ago

I came across this forum post from zodern about types in meteor packages: https://forums.meteor.com/t/zodern-types-tooling-for-creating-and-using-types-from-packages/58369

And I think this seems like the most straightforward way to do it. Using DefinetelyTyped comes with a lot of extra work and testing, which I don't think is worth doing now that Zodern has build this package to easily create typings for a package.

Here Zodern gives an example of how he added types to the old flowrouter: https://github.com/monti-apm/flow-router/compare/c8a53d67ab1b2648c3442677ee52ee512a595b5d...monti-apm:flow-router:0350cd12fad1766b86db23e3fc60d62749555060

I would be interested in helping out, not sure what the best approach would be. Can we use the same typings as Zodern added, or do we create our own based on flowrouter-extra?

dr-dimitru commented 2 years ago

Hello @timsun28

I would be interested in helping out, not sure what the best approach would be. Can we use the same typings as Zodern added, or do we create our own based on flowrouter-extra?

Thank you for the willing to contribute, let's do it! Lets start with PR

dr-dimitru commented 2 years ago

@timsun28 most of the definitions from monti-apm:flow-router should work with FR-extra; But may require "extra" definitions 😅

timsun28 commented 2 years ago

@dr-dimitru I have opened a pr for the types for flowrouter extra.

Let me know what you think and how we can get this merged once it is working as expected. (this is one of my first open source contributions, so sorry if I missed something :smile: )

dr-dimitru commented 1 year ago

@afrokick @timsun28 Is there a way to cover it with tests?

timsun28 commented 1 year ago

@dr-dimitru

To keep the discussion going here, i've copied my message from the merged pr:

I have taken a look at the other meteor packages that have implemented the same zodern package and it seems like they are already completely developed in typescript itself.

I read some more about general ways of testing typescript and I think the following article describes it well: https://developers.mews.com/why-we-should-all-be-testing-our-typescript-types/ I think testing it with typescript itself would be most interesting to add. This would mean changing the test files to typescript files and see if there are any unexpected errors, and possibly add some tests to describe cases where typescript should give an error.

However, I've tried converting a .spec.js file to .ts, but it doesn't seem to recognize the types in my vscode. I've tried adding typescript as a npm dev dependency, and i've added a tsconfig.json file, but without any luck, unfortunately.

As an example of how DefinetelyTyped is tested you can see the meteor package: https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/meteor/test And one of the apps mentioned by zoder already has all their code in a .ts file like this: https://github.com/wildhart/meteor.jobs https://github.com/meteor/react-packages/tree/master/packages/react-meteor-accounts https://github.com/zodern/meteor-pure-admin

zodern commented 1 year ago

For zodern:relay I am using tsd for testing the types. The tests for the code are still written in js. It probably is possible to write them in typescript, but I didn't see a need to.

dr-dimitru commented 1 year ago

Closing this one as conversation is logically complete and v3.9.0 is out. Feel free to create separate issue regarding testing for TS definitions