Making it possible to use angular2-logger with Angular Universal

[FelipeTaiarol]

Before this change store (and therefore localStorage) was being used even if the 'store' option was false as long as a store had been persisted before. This behavior was changed so that now the store will only be used if the 'store' option is true. This will make it possible to use angular2-logger with Angular Universal by providing the custom options { global: false, store: false }.

[FelipeTaiarol]

@langley-agm , can you please let me know if you are going to merge this pull request ?

[langley-agm]

Hi @FelipeTaiarol, thanks for following up with this. I've been sick this week, but If I get any better over the weekend i'll take a look to it.

[FelipeTaiarol]

Cool, thanks!

[langley-agm]

Btw @FelipeTaiarol

Before this change store (and therefore localStorage) was being used even if the 'store' option was false as long as a store had been persisted before.

Is not "a store" persisted before, is the "store" option activated before. That's done on purpose. If you activate it and you have the persistence turned on, it will persist this setting, so you don't have to activate it again every time you refresh the page. If you turn persistence off using the api, it will not get activated upon refresh.

[FelipeTaiarol]

Hmm, I see what you're saying. Even if the store configuration is false if the store level was persisted before you want it to be used. Let me see if I can make a change that will make it work with Universal and not remove that behavior.

[FelipeTaiarol]

@langley-agm , please take a look now.

[langley-agm]

@FelipeTaiarol Added some comments, however, have you tested that this change don't make messages never appear when they should? It sounds to me that if they get sent while running in the server they'll be lost and never get to the user.

This might be helpful:

https://github.com/angular/universal/issues/272

[FelipeTaiarol]

@langley-agm , I have tested it and the messages are shown in the server the same way they are in the client. They will be shown the same way a console.[log|debug|error] is shown when executed by a NodeJS application.

[langley-agm]

@FelipeTaiarol I'm not worried about showing them in the server, this is a client side framework, what I'm worried about is if when they get executed in the server, the client never sees those messages? Do they get ran both in the server and the client? or some in the client and some in the server? Curious though, how do they get ran in the server if the server can't access the storage to see if its turned on?

[FelipeTaiarol]

@langley-agm , with universal the javascript code only runs in the server for the first page load, after that the application continue working as a normal Single Page App. When the ng2-logger code runs in the server the only way the developer can see it is by seeing the NodeJS console (or log file or whatever was configured). The logs which are printed in the server will not be available in the client. After the first page load the code will run in the client and ng2-logger will continue working normally.

[langley-agm]

@FelipeTaiarol That's what I'm trying to say, those messages will be lost to the user of the browser and that's not the desired behavior. Server side errors will usually have their own logging functionality, the end user will expect to see client errors in the browser and it won't be intuitive for them why the messages aren't showing.

[FelipeTaiarol]

@langley-agm , they will not be lost, they can be seen in the server logs. Same thing with debugging, if you want to debug the UI code running in the browser you use the browser and if you want to debug the UI code running in the server you use a debugging tool for the server.

[langley-agm]

@FelipeTaiarol This library is targeted for the browser users, not all of them will have access to the server, they will be lost to them.

[FelipeTaiarol]

@langley-agm , no, there is some confusion because you are probably not familiar with Angular Universal. A UI developer using Angular Universal would always be able to run the server that renders the UI on his local environment. Anyway, my change simply checks if localStorage is available before using it so the library will continue working exactly as before and it will be possible to use it with Angular Universal.

[langley-agm]

A UI developer using Angular Universal would always be able to run the server that renders the UI on his local environment

How about a production environment?

[FelipeTaiarol]

Then the UI developer would need access to the server that renders the UI in order to see the logs. But my point is that even if you want to create a mechanism that would allow a developer to see the logs which were created during the server side rendering in the browser (which I don't think is a good idea) this can be done later. With this change for now it is at least possible to use angular2-logger with Angular Universal, without this change someone that wants to use Angular Universal would have to stop using angular2-logger.

[langley-agm]

Then the UI developer would need access to the server that renders the UI in order to see the logs.

That exactly what Im trying to tell you

not all of them will have access to the server

I understand the problem you are having, I do not agree with the solution you are proposing because it can create confusion to some users.

Also like I said, the user won't be able to turn it on/off during the bootstrapping if they don't have access to the storage, which is one of the main test cases this library was done for. I'd do more research on it for a different solution.

[FelipeTaiarol]

Fair enough, but like I said, currently the user would have to make a choice between Angular Universal and ng2-logger, with this change it is possible to use it and it will work in the browser just like it has always worked. For the server the library doesn't provide a way to change the log level during runtime out of the box, but this is something that can be done later.

[langley-agm]

it will work in the browser just like it has always worked

I'm not sure I agree. I wouldn't call splitting the messages between two consoles creating two different sources of truth and no longer been able to turn it on for good dynamically "just like it has always worked".

[FelipeTaiarol]

This pull request is not doing any of that, it is just checking if localStorage is available before using it.

[langley-agm]

Its like putting an IF before a Null Pointer Exception, it might seem like its fixed because the error won't appear anymore but it doesn't fix the fact that it shouldn't have been null in the first place.

This way the user knows something is being used in a manner it wasn't designed and he can decide to hide the error or deal with it somehow themselves only AFTER they have realized that.

[FelipeTaiarol]

Ok, I'll close this pull request then.