-
Notifications
You must be signed in to change notification settings - Fork 109
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
File/Line numbers? #21
Comments
have a look, I'll make a pull request if this is ok: Changes:
Usage: // Same as before, no changes
this.logger.trace('Hi there'); // Hi there index.js ...
// or .. set 'logImmediately' to false:
LoggerModule.forRoot({
serverLogLevel: NgxLoggerLevel.TRACE,
level: NgxLoggerLevel.TRACE,
logImmediately: false
}),
// and add '()' when using
this.logger.trace('Hi there')(); // Hi there app.component.ts ...
// if 'logImmediately' is not defined or set to true.
// It will log twice. Once inside the logger function, and once on the 'caller' side.
Not sure if it's worth the trouble though, adding '()' can be quite annoying .. and ugly. |
so, was this merged? |
nope, not yet ... So instead of really helping, it could hinder your development (sometimes). Tried looking at how other loggers do it, they don't (without some sort of performance implications - like using the |
yep I just tried by packaging your fork, it works but it feels hacky with the extra (). Without showing the file and line number it's pretty difficult to debug. |
@sam-lex, I'm not sure I like returning a function for this non Here are my thoughts:
So how do we extend the logger with these things in mind:
One idea would be to have a function that builds and returns the arguments that are passed to console.log. You could use the logger to build out the arguments, but call console.log in your own file:
getArgs would return
I realize this would be more code to type than adding () at the end of the logger call, but this new method could be leveraged by the existing logging methods and it would have a single purpose. Also, when using it this way, you wouldn't accidentally forget to add the (), but instead it would be very clear what arguments the method needs to complete its task. Your thoughts? |
Yeah, adding Since my javascript knowledge is very limited, I'll have to research more before I can suggest another solution. One idea that I got, but have no clue how to do it, or if it's even possible: Override the Reason, it's possible to return a "formatted" console.log. which is perfect except that it won't log to the server. So if it's possible to somehow chain the |
I found some stack overflow documents (related to stack trace / caller function tracking)
We can trace caller function by
There is some method to find out the implement file name & position from function reference.
To support this method properly with all browser, it might be require some additional dependency. |
@sam-lex, I don't think we should go the route of overriding console.log. It would never be a good idea to do something like this. Since console is shared globally, it would not just make the change for your code, but also for every other module that you've imported. It could have some pretty gnarly side effects. @leo6104, It appears that arguments.callee.caller is deprecated. |
@dbfannin 😭 Okay, i try to find another way. Thank you for giving updated information. (Korean references are not up-to-date translations.) |
Any news on this? Seems to be some kind of common problem in JS as I could find out through Google search. |
not on my side, I've been using my own work-around version, but is not really useful to me anymore, it's taking a long time to build my angular project. angular/angular-cli#5775 (comment) so with |
Hi Guys, @dbfannin your idea about the args is great, and it actually has many more use cases. It resembles my favourite GoLang logging library: https://github.com/Sirupsen/logrus The syntax I normally use goes like this: |
I'm currently testing some logging libraries and loglevel was the only one that solves the "File/Line numbers" problem. Maybe it can be used as an inspiration ;) |
@stan187, the library loglevel has a slightly different use case. They do not format or modify the console text at all, and they also don't support logging on serverside. Because of this, they can bind their methods to console. Unfortunately, we can't do that. However, since we're still having a lot of pull for this feature, I've made some changes so that we can log the filename and line number along with the logs. After #56, this feature should be supported! |
loglevel has a plugin for server side logging |
filename:line number is now supported in [email protected]. Hopefully this helps! |
Hi @dbfannin , I just upgraded one project from Angular CLI 1.7.4 with ngx-logger: ^2.2.4 to Angular CLI 6.0.3 with ngx-logger ^3.0.0-rc1 and filename:linenumber do now reference to main.js all the time when i run Example: |
@JavierFuentes have you solved this |
@TeXnicians follow #69 for updates. |
For me the line numbers work if I add
|
Doesn't seem to work in the latest Angular version 7.2.x |
@Zenguro I am on 7.2.2 - must be something else. But as one would expect, |
Back again. Now using Angular 8. The option is still deprecated but |
Currently the line numbers are lost: showing as
index.js
which points to where the console.log is being calledThis picture shows as
vendor.bundle.js
because I had--sourcemap=false
on.I did managed to get it to show the "correct" file/line like so:
But with a major downside, and that is having to make the call with extra
()
.To avoid breaking the api, we could set a flag to make it either log directly or return a function.
I can't think of a "pretty" way of doing this though. Any thoughts?
The text was updated successfully, but these errors were encountered: