Add TypeScript types

[jankapunkt]

We need to add TypeScript types, if possible as part of the build chain.

An example would be

"build": "rollup --config rollup.dist.config.js && tsc",

with the following tsconfig.json

{
  "include": [
    "dist/ThinStorage.js"
  ],
  "compilerOptions": {
    "declaration": true,
    "emitDeclarationOnly": true,
    "allowJs": true
  }
}

However, I'm still undecided whether to add tsc as devDependency or require it globally (which then would imply updating our CI workflows to make tsc available for the build step)

[shrihari-prakash]

Hello @jankapunkt ,

First of all, nice template to start an NPM package. I am considering this template to use on a project related to the framework I built on top of node-oauth-server project. I already have this package that is very basic yet does the job for now, but want to upgrade to this to add some additional functionalities to it. I just want to ask, is it not possible to add types to this at the moment? If not, how do you handle IDE autocompletes with this package currently?

[shrihari-prakash]

Normally I do not install any dependencies to add types and simply have an index.d.ts in the root of the project that has all the type definitions and reference it in the typings key of package.json. Could the same not be done here?

[jankapunkt]

Yes you could easily add an index.d.ts and I think we could even add tsc as dev dependency and a script to build typescript types from source. Are you experienced with that?

[shrihari-prakash]

I tried to publish a package with this yesterday and I used the same technique you mentioned in the description of this issue (adding tsc as a dev dependency and using it in build command). When I added JS docs to all my code, the resulting index.d.ts file was perfect and usable 🙂. See output here: https://github.com/shrihari-prakash/liquid-node-authenticator/blob/master/dist/index.d.ts. The thing is though, I changed the rollup config to generate a sindle UMD module instead of separate files for CommonJS and ES modules.

[jankapunkt]

I tried to publish a package with this yesterday and I used the same technique you mentioned in the description of this issue (adding tsc as a dev dependency and using it in build command). When I added JS docs to all my code, the resulting index.d.ts file was perfectly generated and usable 🙂.

Great 🚀 if you want you can open a PR for this 🙏

The thing is though, I changed the rollup config to generate a sindle UMD module instead of a separate files for CommonJS and ES modules.

Yeah I am still unsure which one is better. For now I mostly sticked with the single files because you can map them for each system as main entry point or for CDN to use, however I currently think minification is even of more importance here. So until I have a definitive idea how to move on with rollup config I would keep it as it is, please.

[shrihari-prakash]

Sure, I will raise a PR later today.

About the UMD, yes, you do lose minification here. But the main reason I did this is because my projects were ultimately compiled into CJS (which I think most projects do) though written in ES6 TypeScript. I was having a lot of import problems. The main attribute of package.json was pointing to the CJS version, but node was always importing the plain index.js and erroring out stating you can't use import statements outside of modules.

[jankapunkt]

Hm I think we should have another issue for this one 🤔

[shrihari-prakash]

@jankapunkt I opened a PR implementing what we discussed.

Also, any reason why you commit the dist folder also? As I am aware, CDNs index from NPM and not GitHub?

[shrihari-prakash]

@jankapunkt do not review that PR. I just realized that there are problems with auto generated stuff. Especially when you use classes. I think it is better to write the types on your own and simply copy them to dist on npm run build. Like this:

"build": "rollup --config rollup.dist.config.js && copyfiles -f lib/index.d.ts dist"

Where copyfiles package can work with both windows and linux without throwing errors for a missing cp command.