search

oozcitak / xmlbuilder2

An XML builder for node.js
366 stars 36 forks source link

move exsiting dependnecy `@types/node` from run-time to build-time #165

Closed jkowalleck closed 1 year ago

jkowalleck commented 1 year ago

Describe the bug

the package currently requires @types/node for RUNTIME. this appeases to be an error.

therefore, every productive installation of this package will install @types/node -- for what reason?! Caused by https://github.com/oozcitak/xmlbuilder2/blob/9443dbfe0f18471903c0a7e08912a1c0221fc506/package.json#L33

To Reproduce install this package without any dev nor optional dependencies, as a downstream dependency, and you will find @types/node installed

Expected behavior no dev-dependencies are installed downstream.

Version:

Additional context

@types/node is a dev dependency, that provides type hints digested by developer tools and TypeScript, on build-time. It is not required on run-time.

jkowalleck commented 1 year ago

I read https://github.com/oozcitak/xmlbuilder2/issues/84#issuecomment-810294400 and this wrong. @types/node does not bring any runtime-functionality to the table. And has therefore no effect at runtime, and is no runtime requirement. It is a devDepencency of this very package. If downstream users want to use typescript, they will have the appropriate dependency to set downstream.

I read https://github.com/oozcitak/xmlbuilder2/issues/52#issuecomment-683636799 and this appears to be wrong. It exposes no types at all, as this is no capability of nodejs in the first place. Instead, @types/node provides type hints that might be optionally digested downstream (by tools in dev-time OR by TypeScript on build-time)

Please see https://www.npmjs.com/package/@types/node?activeTab=code's package.json you will find that there is not even an entry point nor any *.js file in the package. @types/node has no effect on runtime. And shall therefore be removed from runtimeDependencies.

jkowalleck commented 1 year ago

Possibly related: #153

universalhandle commented 1 year ago

Hi, @jkowalleck. Thanks for the issue and pull request. I'm inclined to agree that @types packages need only be dev dependencies. I'm going to revisit the threads you linked just to be thorough, but I expect I'll be merging and shipping it.