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

Typescript support #358

Open
guyisra opened this issue Feb 3, 2020 · 25 comments
Open

Typescript support #358

guyisra opened this issue Feb 3, 2020 · 25 comments

Comments

@guyisra
Copy link

guyisra commented Feb 3, 2020

nearly 3 years ago, #202 (comment) mentioned typescript won't be supported

Since then typescript has gained momentum, community and support by many in the js community (https://2019.stateofjs.com/javascript-flavors/typescript/)
While typescript is made by mozilla's sworn enemy, as programmers we strive for best of breed tools. Typescript is arguably a tool that many teams choose to use due to its abilities.

Typescript's support is currently done unofficially through community effort on https://github.com/DefinitelyTyped/DefinitelyTyped but it will make it so much easier to use node-convict if there will be official support for it within the project.

Please reconsider having typescript support.

@A-312
Copy link
Contributor

A-312 commented Feb 3, 2020

Please explain what we need to do to support Typescript

@guyisra
Copy link
Author

guyisra commented Feb 15, 2020

essentially, all that is needed is just a file with all the definitions of all the public / exported functions and types used by them. thank you 🙏

@JoshuaToenyes
Copy link

@guyisra I've been using @types/convict for typing for years. It's not as nice as having the types available in the same package, but better than nothing!

@vadistic
Copy link

vadistic commented Mar 25, 2020

I just wanted to leave few ideas around, since this thread seems like a good opportunity.

There's a bit of issue with typing nested config variables (using @types/convict), because of impossibility of typing dot access notation.

Example:

export const CONFIG = convict({
  parent: {
    child: {
      doc: 'smth',
      enc: 'SMTH',
      type: String,
    },
  },
})

const typescriptGibberrish = CONFIG.get('parent').child

const  typedAsAny = CONFIG.get('parent.child')

I would propose for consideration expanding get/has/default apis to variadic fn that would accept separate path segments.

const val = CONFIG.get('parent', 'child') // val is correctly typed as string
  • implementation would be trivial (smth like args.length > 1 ? args.join(".") : args[0])
  • can be done without any breaking api changes
  • would allow strongly typing nested config variables using mapped types.
  • (frankly - there would tiny issue with typing set method this way, because as far as I can tell - it would require value as first arg - imo I would leave this method as is)

As I wrote - implementation would be quite trivial - the question is wherther it's good idea :)

On the other topic:

@A-312 I would suggest you to include definitelyTyped typings in you're core repo. It's not that big thing, but my main reasoning is - that even plain js users who would never download @ types typing will benefit from improved method intellisense in editors such as vscode.

It's as simple as:

  1. it's MIT, but I would suggest to let the typings author know about it :)
  2. include index.d.ts in your src
  3. copy this file to dist/build in your build chain (even cp src/index.d.ts dist will do)
  4. include "types/typings": "dist/index.d.ts" key in your package.json

Cheers!

@booninite
Copy link

@vadistic when we use config.getProperties(), the generics in @types/convict allows for autocomplete and type-ahead. Additionally, you can do stuff like ReturnType<typeof config.getProperties> to crystallize the types (e.g., to use them as input parameters to a function, or any other useful stuff)

@A-312
Copy link
Contributor

A-312 commented Apr 11, 2020

@vadistic @booninite Where is the doc about that ? I want to implemente typescript support

@xamgore
Copy link
Contributor

xamgore commented Apr 29, 2020

@A-312 you have two options:

  1. Write in the README about the @types/convict package. A user just needs to install it via npm install @types/convict --save-dev.
  2. Copy those type definitions into this repository and set types: index.d.ts in the package.json.

@madarche
Copy link
Collaborator

@xamgore could you make a PR on this please?

@A-312
Copy link
Contributor

A-312 commented Apr 29, 2020

@xamgore My comment was to make a PR, not for use with TS.

@HitkoDev
Copy link

@JoshuaToenyes (+ @madarche and everyone else): Convict split up across 3 packages, and changed a few other things along the way. As this seems to be the way to go, let's be real, we can't expect the community to keep ever increasing number of typings up-to-date.

Considering #202 (comment), if providing typescript integration isn't desired, could Convict at least provide proper type hinting and switch to ES modules already, if you claim to prefer standard ES?

@A-312
Copy link
Contributor

A-312 commented May 11, 2020

Do you realize that you ask a typescript API to people that don't use TS? Split 3 packages that was a little change (some format are now optionnal, is not a big change on the convict interface)

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/convict/index.d.ts

@HitkoDev
Copy link

HitkoDev commented May 11, 2020

Do you realize that you ask a typescript API to people that don't use TS? Split 3 packages that was a little change (some format are now optionnal, is not a big change on the convict interface)

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/convict/index.d.ts

Do you realise you could provide type hinting and ES modules without using typescript? But despite the claim that this project prefers to go with next (standard) versions of JS, no such thing has been implemented in the past 3 years since those claims were made.

@madarche
Copy link
Collaborator

@HitkoDev when you mention that convict could provide type hinting without using TypeScript, do you mean providing JSDoc for public methods? Be specific please. And when you mention ES modules, what concrete advantages will this provide right now with respect to type hinting? Node.js has just moved ES modules out of experimental, so it's unfair to accuse convict in this regard.

So instead of complaining, please propose PRs (like @A-312 does) and we can discuss on something concrete.

Finally, about TypeScript, it's been some time since #202 (comment). And TypeScript track record hasn't been bad since. And it's possible to imagine that if convict could be written in TypeScript in the future, only some parts of convict's code (the public API) could be strictly typed, while the internals could be let in not-strictly-typed JavaScript. So that would please the outside-developers, while still be pleasant for the inside-developers. But I would prefer not to consider this option before we make convict's code even more maintainable, see #370

@xamgore
Copy link
Contributor

xamgore commented May 17, 2020

There is no need to rewrite the codebase in typescript. Just a index.d.ts declaration file is enough.

@HitkoDev have you seen @types/convict package?

@A-312
Copy link
Contributor

A-312 commented May 17, 2020

'just' for no typescript dev: Take a day to understand how that work and write file, then try to use this file. You ask a lot of, I think

Are you able to do this PR on your own ?


@petermetz Hi, I saw you are the last person to commit on convict/index.d.ts files. Are you interested to add the files & test directly here on convict package ? Is it possible ? (I can't do on my own (that will take me to many time to do that change) I don't use typescript, and seem person that ask this feature don't want to make this change)

@petermetz
Copy link

@A-312 Sorry but I'm stretched thin as it is. I only contributed a few lines to the existing typings because they were badly needed for my case. I understand that it's more comfortable for newcomers to have the typings embedded with the library itself but I also think it's not a big deal having to install a separate dev dependency for the typings to justify significant effort on my side.

@xamgore
Copy link
Contributor

xamgore commented May 21, 2020

At the moment, I'm migrating a project to convict 6, so I need some time to investigate the changes and rewrite typings. @DefinitelyTyped repo has CI with test runners and experienced maintainers, so I'm not sure it's a good idea to pull the blanket.

@A-312
Copy link
Contributor

A-312 commented May 22, 2020

At the moment, I'm migrating a project to convict 6, so I need some time to investigate the changes and rewrite typings. @DefinitelyTyped repo has CI with test runners and experienced maintainers, so I'm not sure it's a good idea to pull the blanket.

Keep in your head that this major (v6) has not so many change, don't past too many time on. The next version will be more interesting.

@tomasz-zablocki
Copy link

Current typescript beta (4.1) brings template literal types and key remapping. They seem to have a lot of potential and it looks like convict types are a prime candidate for adopting these two.

@MickL
Copy link

MickL commented Mar 15, 2021

IMO it would be more convenient if a "npm install convict" comes with everything. Using @types is becoming out of fashion and exists mostly only for old libraries. Also I think that the maintainers of this project knows their interfaces the best so they could easily maintain the typings while working contributing, over @types it is often hard to find anyone to maintain or review or sometimes a package gets updates but @types is not updated in the same commit/release.

@samlevin
Copy link

Incredible how vocally opposed this maintainer is to the idea of adding native TS support to a mainstream library, and for seemingly no good reason? TypeScript is everywhere, I can't imagine working without it. Every year we're depending less and less on @types as most of the community is supportive of this. I suppose I'll work towards replacing convict altogether with TypeScript-native functionality to the furthest extent possible. That, or looking for a modern alternative that caters to the needs of its users

@digimbyte
Copy link

You can generate types automatically with tsc --declarations
it's documented here: https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html

@brianjenkins94
Copy link

I'm working on a TypeScript rewrite 🙂

@madarche
Copy link
Collaborator

madarche commented Dec 1, 2021

@brianjenkins94 that is a constructive proposition! 👍

@brianjenkins94
Copy link

brianjenkins94 commented Dec 5, 2021

I'm still trying to grok and rework some of the dense logic, but here's where it will be if anyone wants to help!:

https://github.com/brianjenkins94/juvy / https://www.npmjs.com/package/juvy

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

No branches or pull requests