feat(core): introduce Logger service by DzmitryShylovich · Pull Request #14308 · angular/angular

DzmitryShylovich · 2017-02-05T09:27:00Z

Closes #5458

FrozenPandaz · 2017-02-05T17:24:52Z

Thoughts on implementing Logger.time as console.time?

https://developer.mozilla.org/en-US/docs/Web/API/Console/time

chunghha · 2017-02-08T16:31:45Z

console.group as well please.

https://developer.mozilla.org/en-US/docs/Web/API/Console/group

mhevery · 2017-02-09T20:09:15Z

I believe all JavaScript platforms have console. I that is the case we should move Logger into core and the default implementation should just do console.log

What do you think?

gkalpak · 2017-02-09T21:16:15Z

IE9 might need special treatment, because console is only available when DevTools are open (and has some quirks, like console.log not inheriting from Function.prototype).

(Just pointing out that special treatment might be needed, not implying that it shouldn't be done.)

DzmitryShylovich · 2017-02-09T21:22:17Z

@gkalpak it's already handled here. Basically it's a function from ng1. If console is not available it will be noop.

gkalpak · 2017-02-09T21:58:56Z

We've been discussing about removing this method from AngularJS.
Stringifying the error, breaks source map supports (see angular/angular.js#15590 (comment) for more info).

Yeah, I'm in the process of removing this function for AngularJS 1.7 for all the browsers with the exception of IE/Edge (as their error logging is far from ideal). Chrome/Safari apply source maps to logged errors, Firefox is in a process of implementing that as well. By adding this function you'll break it and if you use Webpack - which many people will as that's what Angular CLI uses - you'll just see references to app.bundle.js or vendor.bundle.js which makes making sense out of the stack virtually impossible.

Currently this method is not used.

That's true. Can it just be removed then?

DzmitryShylovich · 2017-02-10T12:22:08Z

do we really need this check? afaik ng2 doesn't officially support ie8

Yeah, try-catch shouldn't be needed any more. Guarding against missing console.log.apply is as IE 9 has console method not inheriting from Function.prototype.

DzmitryShylovich · 2017-02-10T12:26:24Z

@mhevery you're right, console is available on on supported platforms.
Moved Logger into core.

Also added group and groupEnd because these methods are used by the router.

mhevery · 2017-02-10T21:03:39Z

rebase please

DzmitryShylovich · 2017-02-10T21:25:26Z

@mhevery rebased

mhevery · 2017-02-13T19:20:18Z

We decided to not merge this in as at is a new feature and @IgorMinar and I need to debate whether we want to add and support this kind of features.

vicb · 2017-02-14T23:39:27Z

Please remove @param args and make comment 1 liners

vicb · 2017-02-14T23:41:24Z

private _getConsoleMethod(method, args) and remove all the private wrappers.

but in this case we will need to make unnecessary checks on every invocation.

vicb · 2017-02-14T23:43:36Z

Not sure if the comments are necessary on the Logs prop

there's a similar comments in ng1. I thought it would be useful for generated api docs

vicb · 2017-02-14T23:45:20Z

@mhevery @IgorMinar Yep, it would be good to have it, we use a mic of console.log, Console service today.

Closes angular#5458

trumbitta · 2017-02-15T09:13:34Z

I'm puzzled and can't figure out why a framework wouldn't have a logging system :-/

alastair-todd · 2017-04-05T13:36:48Z

So, 2 months on 1 million people have introduced a nigh-on identical but independent "custom" LoggingService into their apps.

C'mon what's to consider here?

IgorMinar

My primary issue with this PR is that logging typically becomes very expensive (in terms of payload-size) very quickly. Until we have a design that can safely strip non-critical logs and minify the critical ones I think everyone is better off rolling their own solution. The api above would likely need to be changed to achieve these properties, and doing so after these apis are heavily used would be very disruptive and painful for everyone.

The cross-cutting concern of logging and necessity to provide a solution that works well for 3rd party components makes this a much harder problem than what it looks like on the surface and we are better off deferring tackling this problem rather than providing a solution that would send everyone down the wrong path.

splincode · 2018-03-16T17:54:06Z

@IgorMinar Should we close this task?

splincode · 2018-07-26T15:49:29Z

@DzmitryShylovich @mhevery Is the PR is relevant?

mhevery · 2018-07-27T21:23:44Z

Closing per @IgorMinar's comment.

angular-automatic-lock-bot · 2019-09-13T21:21:51Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

googlebot added the cla: yes label Feb 5, 2017

gkalpak reviewed Feb 9, 2017

View reviewed changes

googlebot added the cla: yes label Feb 10, 2017

DzmitryShylovich commented Feb 10, 2017

View reviewed changes

mhevery approved these changes Feb 10, 2017

View reviewed changes

mhevery added the action: merge The PR is ready for merge by the caretaker label Feb 10, 2017

mhevery removed the action: merge The PR is ready for merge by the caretaker label Feb 13, 2017

vicb reviewed Feb 14, 2017

View reviewed changes

feat(core): introduce Logger service

6157924

Closes angular#5458

chuckjaz added area: core Issues related to the framework runtime feature Label used to distinguish feature request from other issues labels May 23, 2017

Toxicable mentioned this pull request Jun 10, 2017

feat(core): introduce Logger Service #17399

Closed

2 of 3 tasks

IgorMinar added the state: blocked label Jul 11, 2017

IgorMinar suggested changes Jul 11, 2017

View reviewed changes

mhevery closed this Jul 27, 2018

angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 13, 2019

Conversation

DzmitryShylovich commented Feb 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

FrozenPandaz commented Feb 5, 2017

Uh oh!

chunghha commented Feb 8, 2017

Uh oh!

mhevery commented Feb 9, 2017

Uh oh!

gkalpak commented Feb 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DzmitryShylovich commented Feb 9, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mgol Feb 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DzmitryShylovich Feb 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DzmitryShylovich commented Feb 10, 2017

Uh oh!

mhevery commented Feb 10, 2017

Uh oh!

DzmitryShylovich commented Feb 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mhevery commented Feb 13, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vicb commented Feb 14, 2017

Uh oh!

trumbitta commented Feb 15, 2017

Uh oh!

alastair-todd commented Apr 5, 2017

Uh oh!

IgorMinar left a comment

Choose a reason for hiding this comment

Uh oh!

splincode commented Mar 16, 2018

Uh oh!

splincode commented Jul 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mhevery commented Jul 27, 2018

Uh oh!

angular-automatic-lock-bot Bot commented Sep 13, 2019

Uh oh!

Reviewers

Uh oh!

DzmitryShylovich commented Feb 5, 2017 •

edited

Loading

gkalpak commented Feb 9, 2017 •

edited

Loading

mgol Feb 9, 2017 •

edited

Loading

DzmitryShylovich Feb 10, 2017 •

edited

Loading

DzmitryShylovich commented Feb 10, 2017 •

edited

Loading

splincode commented Jul 26, 2018 •

edited

Loading