javascript icon indicating copy to clipboard operation
javascript copied to clipboard

Fix missing types

Open danmontgomery opened this issue 2 years ago • 1 comments

This fixes compilation errors introduced when types all moved to devDependencies - described in #883

By default, there will still be some errors because of default imports, which are all fixed by setting esModuleInterop to true (maybe worth documenting, or addressing in a separate PR?)

npm init -y
npm install typescript
npm install @kubernetes/client-node
echo 'import "@kubernetes/client-node"' > index.ts
npx tsc index.ts

Before:

node ➜ /workspaces/k8s-client $ npx tsc index.ts
node_modules/@kubernetes/client-node/dist/attach.d.ts:1:23 - error TS2688: Cannot find type definition file for 'node'.

1 /// <reference types="node" />
                        ~~~~

node_modules/@kubernetes/client-node/dist/attach.d.ts:2:23 - error TS2688: Cannot find type definition file for 'ws'.

2 /// <reference types="ws" />
                        ~~

node_modules/@kubernetes/client-node/dist/attach.d.ts:4:25 - error TS2307: Cannot find module 'stream' or its corresponding type declarations.

4 import stream = require('stream');

...

Found 166 errors in 71 files.

After:

node ➜ /workspaces/k8s-client $ npx tsc index.ts --esModuleInterop
node ➜ /workspaces/k8s-client $ 

danmontgomery avatar Oct 13 '22 14:10 danmontgomery

@danmontgomery thanks for the PR, but from my reading of this npm document:

https://docs.npmjs.com/specifying-dependencies-and-devdependencies-in-a-package-json-file

It seems that @typings files should not be included in the dependency section as they are not required for production, they are only required for development.

I'm open to arguments that I'm wrong, but this is the best source I could find on how package.json should be set up.

brendandburns avatar Oct 20 '22 20:10 brendandburns

@brendandburns That's true, except when publishing a typescript library... "production" in this case are users of the package (anyone adding @kubernetes/client-node to their package.json). With these types not included in dependencies, they aren't grabbed by npm/yarn when installing the package, resulting in the errors posted above (and in the issue I linked to), so developers using this package need to manually add these undocumented dependencies to their own package.json

https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html#dependencies

The alternative (https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html#publish-to-types) would be to submit a PR to https://github.com/DefinitelyTyped/DefinitelyTyped declaring type dependencies :)

danmontgomery avatar Oct 20 '22 21:10 danmontgomery

@danmontgomery thanks for the additional details. I'm ok to merge this.

/lgtm /approve

brendandburns avatar Oct 22 '22 21:10 brendandburns

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brendandburns, danmontgomery

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot avatar Oct 22 '22 21:10 k8s-ci-robot

@brendandburns thanks for fixing both typescript issues (Im referring to the types dependencies). When can we expect a new version? We are eager to start working with the library and this is currently a show stopper for us :-)

rbnayax avatar Oct 23 '22 08:10 rbnayax

Any news on the release?

matteosilv avatar Nov 17 '22 09:11 matteosilv