untu / comedy

Node.js actor framework.
Eclipse Public License 1.0
650 stars 36 forks source link

DEPENDENCY: Winston failing on node 14 #60

Open rijnhard opened 3 years ago

rijnhard commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

weekens commented 3 years ago

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 commented 3 years ago

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

weekens commented 3 years ago

@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.