mozilla / node-convict

Featureful configuration management library for Node.js
Other
2.34k stars 146 forks source link

Typescript support #358

Open guyisra opened 4 years ago

guyisra commented 4 years ago

nearly 3 years ago, https://github.com/mozilla/node-convict/issues/202#issuecomment-297392945 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 commented 4 years ago

Please explain what we need to do to support Typescript

guyisra commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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

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!

shimmerjs commented 4 years ago

@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 commented 4 years ago

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

xamgore commented 4 years ago

@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 commented 4 years ago

@xamgore could you make a PR on this please?

A-312 commented 4 years ago

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

HitkoDev commented 4 years ago

@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 https://github.com/mozilla/node-convict/issues/202#issuecomment-297392945, 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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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 https://github.com/mozilla/node-convict/issues/202#issuecomment-297392945. 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 commented 4 years ago

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 commented 4 years ago

'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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

brianjenkins94 commented 2 years ago

I'm working on a TypeScript rewrite 🙂

madarche commented 2 years ago

@brianjenkins94 that is a constructive proposition! :+1:

brianjenkins94 commented 2 years ago

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