browserify / commonjs-assert

Node.js's require('assert') for all engines
MIT License
293 stars 57 forks source link

Added types. #25

Closed btraut closed 6 years ago

btraut commented 7 years ago

I was struggling to get typescript working with this package. DefinitelyTyped has an entry for "assert", but it doesn't declare "assert" as a module. As far as I can tell, this is because people often conflict between node.d.ts and the use of assert as an external package.

Realistically, node users should use node.d.ts (and node's built-in assert module), and developers of sites and apps that require packing should use your commonjs-assert package along with its own type definition.

This would've all been avoided if this module was actually called "commonjs-assert" on npm instead of just "assert", but we're a bit beyond that.

Anyway, this pull request contains the proper type definitions tightly coupled with the commonjs-assert module which means consumers of the module won't need to add a separate external dependency on @types/assert or any other such type definitions.

calvinmetcalf commented 7 years ago

shouldn't this just be on definitely typed ?

btraut commented 7 years ago

As far as I understand it, DefinitelyTyped exists only because the creators of type definitions are not necessarily (or usually) the same people as the creators of modules themselves. Bringing the type definitions into the module makes it far easier for users to consume them and also ensures that the type definition version matches that of the module (a common problem when using DefinitelyTyped).

Furthermore, DefinitelyTyped already has type definitions under the name "assert", but they're meant for power-assert. DefinitelyTyped also has definitions for node which include "assert", but that also brings definitions for other first-party node modules. It's messy over there, and best if avoided.

calvinmetcalf commented 7 years ago

It sounds like the root of the issue is that defiantly typed has the wrong assert package?

On Mon, Oct 3, 2016, 2:43 AM Brent Traut notifications@github.com wrote:

As far as I understand it, DefinitelyTyped exists only because the creators of type definitions are not necessarily (or usually) the same people as the creators of modules themselves. Bringing the type definitions into the module makes it far easier for users to consume them and also ensures that the type definition version matches that of the module (a common problem when using DefinitelyTyped).

Furthermore, DefinitelyTyped already has type definitions under the name "assert", but they're meant for power-assert. DefinitelyTyped also has definitions for node which include "assert", but that also brings definitions for other first-party node modules. It's messy over there, and best if avoided.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/defunctzombie/commonjs-assert/pull/25#issuecomment-251066182, or mute the thread https://github.com/notifications/unsubscribe-auth/ABE4n5CqatyxkQnzBTaDrf3qJ60ylwNtks5qwM5JgaJpZM4KMSiB .

btraut commented 7 years ago

It seems that way. I'm also adding a pull request on that side.

I still feel as if tighter bundling of code and types is better (ie: this pull request), but that's your call as a maintainer.

btraut commented 7 years ago

See: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/11695