Skip to content

WIP on #23 #26

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

tuvokki
Copy link

@tuvokki tuvokki commented Oct 8, 2018

Preliminary implementation.
Just to see if this is heading the right way. Since I'm really new to Vue and it's plugin system I'd like to know if, beside the rather simplistic approach, this could be heading in the right direction.
The idea behind this is that the logger uses the options provided on install. It looks like this is not really working so I guess that we'll have to provide a new logger instance to Vue after changing the loglevel, but I'm not sure how to, after the instance has already gone through the install-phase.
My guess would be to call the install code block again with the new options:

        if (this.isValidOptions(options, this.logLevels)) {
            Vue.$log = this.initLoggerInstance(options, this.logLevels);
            Vue.prototype.$log = Vue.$log;
        } else {
            throw new Error(this.errorMessage);
        }

But then I think we've already lost access tom the Vue instance passed into the install method.

I guess I'm kinda stuck here lacking the knowledge of Vue and it's plugin system.

@justinkames justinkames self-assigned this Oct 8, 2018
@justinkames
Copy link
Owner

You might have already read this but here is more detail on how to create plugins for Vue.js @ https://vuejs.org/v2/guide/plugins.html

// calls `MyPlugin.install(Vue)
Vue.use(MyPlugin)

The install function is called once, upon calling Vue.use(). But you can expose a function on the Logger ( Vue.$log ) itself to change the internal log level. You are on the right track, but this function needs to be modified so that const logger = {} can modify itself.

  private initLoggerInstance(options: ILoggerOptions, logLevels: string[]) {
        const logger = {};
        logLevels.forEach((logLevel) => {
                if (logLevels.indexOf(logLevel) >= logLevels.indexOf(options.logLevel) && options.isEnabled) {
                    logger[logLevel] = (...args) => {
                        const methodName = this.getMethodName();
                        const methodNamePrefix = options.showMethodName ? methodName + ` ${options.separator} ` : "";
                        const logLevelPrefix = options.showLogLevel ? logLevel + ` ${options.separator} ` : "";
                        const formattedArguments = options.stringifyArguments ? args.map((a) => JSON.stringify(a)) : args;
                        const logMessage = `${logLevelPrefix} ${methodNamePrefix}`;
                        this.printLogMessage(logLevel, logMessage, options.showConsoleColors, formattedArguments);
                        return `${logMessage} ${formattedArguments.toString()}`;
                    };
                } else {
                    logger[logLevel] = () => undefined;
                }
            },
        );
        return logger;
    }

I will have a look at this pull request this week. But feel free to update it.

@justinkames
Copy link
Owner

@tuvokki

Something like this :

Logger = {
    debug () {
        console.log('var')
    },
    info () {
        console.log('var')
    },
    fatal () {
        console.log('var')
    },
   /// use params for new config ? 
    updateSelf (params) {
        this.debug = function () {
            console.log('Overwriting the old debug')
        }
    }
}

Logger.updateSelf()
Logger.debug()

@nomuna
Copy link

nomuna commented Oct 24, 2018

Logger = {
    threshold : 0; // lets say 0 is for debug ...
    debug () {
        if ( threshold)  == 0 ) console.log('var');
    },
    info () {
        if (threshold >= 1) console.log('var');
    },
    fatal () {
       if (threshold >= 2) console.log('var');
    }
}

Logger.threshold =  1;
Logger.debug() // will not show

How about something like this...

Update:

Sorry for being vague... I thought the question was about changing the log threshold while the application is running, not adding new log levels to an already existing logger for which I can't think of a valid use case.

Looking at an application running in a dev-server with INFO log level and then waning to set the level to DEBUG without restarting the dev-server to see more log messages to track down a problem seems like a very valid use case.

Am I misunderstanding something here?

@tuvokki
Copy link
Author

tuvokki commented Oct 26, 2018

I think you're on the right track regarding the desired functionality here. I wouldn't go as far as (dynamically) adding new loglevels here. For my use-case changing the loglevel to DEBUG for the duration of a single session would suffice.

About your proposal; isn't this yet another way to limit (or expand) the output of the logger? Changing the threshold looks like the same as changing the actual log-level. Or am I misunderstanding something here? I mean, if we can change the treshold in a running instance, wouldn't that be the same effort as changing the log-level itself?

@tuvokki tuvokki force-pushed the #23-Change_Loglevel branch from c180071 to fbe375c Compare October 26, 2018 14:26
Implement logger change and test
@tuvokki tuvokki force-pushed the #23-Change_Loglevel branch from fbe375c to 056e6d5 Compare October 26, 2018 14:35
@justinkames
Copy link
Owner

@tuvokki

I think you're on the right track regarding the desired functionality here. I wouldn't go as far as (dynamically) adding new loglevels here. For my use-case changing the loglevel to DEBUG for the duration of a single session would suffice.

About your proposal; isn't this yet another way to limit (or expand) the output of the logger? Changing the threshold looks like the same as changing the actual log-level. Or am I misunderstanding something here? I mean, if we can change the treshold in a running instance, wouldn't that be the same effort as changing the log-level itself?

Yes, the log functions ( eg. this.$log.debug() ) are just simple functions that only console.log the input. Depending on the type of function it also does console.log, console.error or console.warn.

However, these functions are currently initialized on Vue.use(VueLogger) whichs calls :

initLoggerInstance(options: ILoggerOptions, logLevels: string[]) ;

This function only runs once, initialising the Logger object with its functions ( depending on set logLevel in config ). Lets say we pick 'fatal', then this.$log.debug() will be an empty function, that does nothing. Therefore with the current code we would need to override the empty functions to have the desired result of this PR.

The way to go might be to not initialize the ignored levels as empty functions, but with the actual print functionality.

Then inside the print function we can check the current loglevel and make it print or not. The logger object (this.$log) should then expose a function that allows you to modify the logLevel. Eg:

this.$log.setLogLevel('debug')

Does that make sense? What do you think about this approach?

@tuvokki
Copy link
Author

tuvokki commented Oct 27, 2018

Then inside the print function we can check the current loglevel and make it print or not. The logger object (this.$log) should then expose a function that allows you to modify the logLevel. Eg:

this.$log.setLogLevel('debug')

Does that make sense? What do you think about this approach?

That does make sense. It would even enable on-the-fly setting a different loglevel for a component (e.g. in the beforeMount/destroy method hooks) which would only show debug-logging for that one component.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants