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

DEPENDENCY: Winston failing on node 14 #60

Open
rijnhard opened this issue Feb 17, 2021 · 5 comments
Open

DEPENDENCY: Winston failing on node 14 #60

rijnhard opened this issue Feb 17, 2021 · 5 comments
Assignees

Comments

@rijnhard
Copy link
Contributor

when running the basic getting started example the following error appears:

(node:290995) Warning: Accessing non-existent property 'padLevels' of module exports inside circular dependency

followed by a whole trace if you add the --trace-warnings flag.
This was fixed in Winston 2.4.5 but they do recommend moving to Winston 3.

That said, shipping with a logger is crossing concerns...
Libraries shouldn't have baked in loggers, but they can have interface to configure your own logger. for example you can pass a log function handler

import comedy from 'comedy';

comedy.setLogger((category, level, ...msg) => { /*handle however I want*/ });
// or 
comedy.setLoggerFactory((category) => (level, ...msg) => { /* create a logger for each category*/ });

// then the rest of the usage stays exactly the same.
@rijnhard
Copy link
Contributor Author

as a side note, if you want to still maintain full backwards-compatible behaviour, then you can detect if Winston is already installed and then preconfigure it as the default log handler, otherwise just default to console.

@weekens
Copy link
Contributor

weekens commented Feb 18, 2021

Hi @rijnhard ! Thank you, that's a very good point! I'll implement that on weekend.

@weekens
Copy link
Contributor

weekens commented Feb 18, 2021

I've released a quick fix for Winston as you mentioned. The fix is available in version 2.1.4.

The logging configurability feature is still to come.

#61

@rijnhard
Copy link
Contributor Author

@weekens you are fast! I seldom get such a response from maintainers! thank you

@weekens
Copy link
Contributor

weekens commented Feb 18, 2021

@rijnhard Well, the fix was trivial and you did 100% of the research yourself! :)

The logging configuration functionality won't arrive that fast, I'm afraid :) But that's a needed feature, so I'll be on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants