Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added types. #25

Closed
wants to merge 1 commit into from
Closed

Added types. #25

wants to merge 1 commit into from

Conversation

btraut
Copy link

@btraut btraut commented Oct 3, 2016

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
Copy link
Collaborator

shouldn't this just be on definitely typed ?

@btraut
Copy link
Author

btraut commented Oct 3, 2016

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
Copy link
Collaborator

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
#25 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABE4n5CqatyxkQnzBTaDrf3qJ60ylwNtks5qwM5JgaJpZM4KMSiB
.

@btraut
Copy link
Author

btraut commented Oct 3, 2016

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
Copy link
Author

btraut commented Oct 3, 2016

@btraut btraut closed this Oct 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants