Export Logger class

[Faithfinder]

This allows to extend Logger with custom functionality.

fixes #1902
fixes #2170

[Faithfinder]

I can see that somebody let the tests run here, and they failed because of failed coverage percentage. Is there anything I can do?

[wbt]

Add a semicolon on the end of the line you added, and consider adding a test that you can do the export/extension that you want so future changes don't accidentally revert the functionality you're trying to introduce.

[Faithfinder]

Done. For some reason the coverage didn't increase a tenth of percent 😢

[wbt]

I'm not sure what's causing this check to fail, or if someone recently changed a configuration there. Your second commit helps, but I don't think the PR contents are what trigger the test failure.

In any case, the failing report includes notes about which lines aren't covered; if anyone wants to open PRs to cover those we might be able to get them merged into master and rebase this one by merging from master to here.

[bogdan]

In order to make logger class extensible, please change the return type in add, remove, clear, close, profile and child methods to this (this is special feature of typescript):
https://github.com/winstonjs/winston/blob/master/index.d.ts#L124

It would be good to add a test that checks that child method produces an instance of custom logger class instead of generic Logger.

[vantaboard]

Any updates on this? I would love to have this.

[Faithfinder]

Didn't have time to address the latest (reasonable) comment + I found that .child doesn't actually work as intended (there's a hell of what this points to in the derived class when using .child)

[txj-xyz]

Are there any updates on this coming soon?

[marc-wilson]

Any plans to merge this?

[vantaboard]

For anyone who uses https://www.npmjs.com/package/patch-package. This gist does the trick if you save it to patches/winston+3.8.2.patch.
https://gist.github.com/vantaboard/02f4c1d29115337cad148dba683c80d9

[DABH]

I am fine with merging this if CI passes. However, I don't see any way to re-run the tests. It's possible it's been too long and re-running is not allowed. Perhaps @Faithfinder can push an empty commit or something in order to re-trigger the CI? Or an interested community member could open a new version of this same PR and get the tests to all pass?

[bogdan]

We should also fix the Logger type definition. There are methods that return itself and therefore their return type should be changed from Logger to this. Methods: add, remove, clear, close, profile, child.

[Faithfinder]

Done & done :) Also put a note into README cautioning about .child problems while extending.

[DABH]

Oh wow, thanks for the fast turnaround after the long delay on my end :) Let’s see if we can get CI passing now (or debug what’s wrong with it) and get this shipped…

[DABH]

Oh, the current CI failure might just be due to insufficient test coverage? Let’s add some small test somewhere? 😅

[DABH]

@Faithfinder can you try to merge master into your PR branch / rebase and see if tests pass now?

[Faithfinder]

Done. It seems the workflow needs approval?

Can we do a v4 that is a typescript rewrite with child reimplementation friendly to extending 🤣 ?

[DABH]

@Faithfinder I’m open to that 😅 In the meantime are you able to take a look and see why coveralls still reports failure? Is there some other coverage config file in the repo we can tweak, or do I need to look at the coveralls project config? Oh and one merge conflict came up, my apologies… hoping we can get this merged in asap!

[DABH]

@Faithfinder Were you able to take a look at this? Would love to merge whenever you have time to check the coverage and fix the conflict!

[Faithfinder]

Finally, lol =P