Download / ulog

The universal logger
https://ulog.js.org
MIT License
88 stars 19 forks source link

simplify `create` func #26

Closed johnd0e closed 4 years ago

johnd0e commented 4 years ago

fix #24


This change is Reviewable

Download commented 4 years ago

@johnd0e Thanks for your willingness to contribute! Please have a look at anylogger, it is where this code will live. If you are able to reproduce the problem with anylogger, please create a PR for that project instead.

I am working on the next beta of ulog, it is coming very soon. It is a major refactor so PR's for this project are difficult to merge atm. Sorry about that. But rest assured that this problem will not be in ulog in the next beta. It will either be solved, or be a problem in anylogger, but not here.

johnd0e commented 4 years ago

It is a major refactor so PR's for this project are difficult to merge atm.

It's ok, as currently I'm just trying to quick-fix only this small issue. I see tests failed because they are trying to use name with dashes (which is not valid js identifier). Could you explain this case?

Download commented 4 years ago

@johnd0e Sorry for the late response, I was on a holiday. I am using the Function.prototype.name property to hold the logger name. And indeed, functions can have names that are not valid js identifiers.

e.g.:

var myObject = {
  ['my-function:with:invalid-js-identifier']: function(){}
}

var fn = Object.keys(myObject).map(key => myObject[key])[0]
console.info(fn.name) // prints "my-function:with:invalid-js-identifier"

Because the logger returned by anylogger() or ulog() is a function, using the function.name property seems natural.

The reason I am testing whether loggers with such names are supported is because existing popular packages like debug actually promote such names through their naming conventions and guidelines. So I think ulog must support it to.

johnd0e commented 4 years ago

I am using the Function.prototype.name property to hold the logger name.

Is it necessary to use the name property for compatibility with debug? Or you can pick any, e.g. loggerName?

Where that is actually used later?

Download commented 4 years ago

@johnd0e No it is not strictly needed to use Function.prototype.name, however since that function is the logger, I think it is most elegant if the function name corresponds to the logger name. In modern browsers this can be achieved pretty easily with code like:

var name = 'logger-name'
var logger = {[name]: function(){}}[name]

but we have to do some tricks to support older browsers here.

It is not actually used anywhere in ulog at the moment, but later on I will add e.g. formatters that will print the name of the logger etc.

johnd0e commented 4 years ago

but we have to do some tricks to support older browsers here.

I have to state that current tricks are too dirty, and it'd be more straight just to pick other name.

Thank you for explanations. I am watching for updates.