search

sindresorhus / hook-std

Hook and modify stdout and stderr
MIT License
54 stars 12 forks source link

Add TypeScript definition #20

Closed Codex- closed 5 years ago

Codex- commented 6 years ago

Removed my request from DefinitelyTyped and migrated following your guide.

Hopefully this meets the requirements :)

sindresorhus commented 6 years ago

Let me know if there's anything in the style guide that could be clearer or improved. It's still a work-in-progress :)

Codex- commented 6 years ago

Re: the guide. I suppose the main thing I found a little tricky at first was how to use the type assertions correctly, I ended up going through a few other projects to understand the usage more correctly.

sindresorhus commented 6 years ago

Also note:

For packages with a default export, use export default foo;, not export = foo;. You will have to add module.exports.default = foo; to the index.js file.

sindresorhus commented 6 years ago

You also left out types for the default export.

Codex- commented 6 years ago

Yup I've come to realize this after the comment around the stream options, will sort out soonish :)

Codex- commented 6 years ago

I've hopefully this time covered everything :)

Codex- commented 6 years ago

Mistakes were made...

Codex- commented 6 years ago

Should be good now.

Codex- commented 6 years ago

Actually I take it back, few issues to look at still, will comment when ready for another review

Codex- commented 6 years ago

Updated with the default export and fixed some typings. I think this is more up to scratch now. Sorry for the delays, uni has been crazy busy.

Can you please clarify a couple of things:

If the parameter description just repeats the parameter name, leave it out. Does this mean I should remove all docstring params that are simply @param transform or that there shouldn't be a description if the description just repeats the param such as @param transform the transform function.

In the readme it specifies unhooks both stdout and stderr but if you use custom or additional streams it unhooks these too, should it in that case read something more along the lines of unhooks both stdout, stderr or any specified streams

Cheers

sindresorhus commented 6 years ago

Does this mean I should remove all docstring params that are simply @param transform

Yes

In the readme it specifies unhooks both stdout and stderr but if you use custom or additional streams it unhooks these too, should it in that case read something more along the lines of unhooks both stdout, stderr or any specified streams

Maybe just:

Unhooks the streams in the streams option.

Codex- commented 5 years ago

Just wondering if there were any pending changes you were wanting that I have possibly missed?

sindresorhus commented 5 years ago

Thank you :)

Codex- commented 5 years ago

Great, thanks Sindre!