Skip to content
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

Loglevel 2.0 #119

Open
kutuluk opened this issue Jan 31, 2018 · 9 comments
Open

Loglevel 2.0 #119

kutuluk opened this issue Jan 31, 2018 · 9 comments

Comments

@kutuluk
Copy link
Contributor

kutuluk commented Jan 31, 2018

Loglevel is good. It's time to make it fresh. More clearly. More modern.

  • Remove support for legacy browsers.
  • Remove support for persistance levels.
  • Remove noConflict()
  • Сustomizable levels
  • Flat flow of plugins (only one extra line in the stacktrace for any number of plugins)
  • log.getLogger('child') => log('child')
  • getter/setter log.level
  • maybe something else...

Implementation loglevel.js

const noop = () => {};
const loggers = {};
const configs = {};
const chain = [];
let levels = ['trace', 'debug', 'info', 'warn', 'error', 'silent'];
let levelsInfo = levels.slice();

class Plugin {
  constructor(name) {
    this.name = name;
  }

  // eslint-disable-next-line class-methods-use-this
  factory() {
    return () => {};
  }
};

// Build the best logging method possible for this env
// Wherever possible we want to bind, not wrap, to preserve stack traces
function defaultFactory(methodValue) {
  /* eslint-disable no-console */
  if (typeof console === 'undefined') return noop;

  const methodName = console[levels[methodValue]] ? levels[methodValue] : 'log';
  if (console[methodName]) return console[methodName].bind(console);

  return noop;
  /* eslint-enable no-console */
}

function pluginsFactory(methodValue, logger) {
  const methods = [];

  chain.forEach((plugin) => {
    const rootConfig = configs[plugin.name][''];
    const loggerConfig = configs[plugin.name][logger.title];
    if (rootConfig || loggerConfig) {
      methods.push(
        plugin.factory(
          methodValue,
          logger,
          Object.assign({}, plugin.defaults, rootConfig, loggerConfig),
        ),
      );
    }
  });

  const native = defaultFactory(methodValue);

  return (...args) => {
    for (let i = 0; i < methods.length; i++) {
      methods[i](args);
    }
    native(...args);
  };
}

let factory = defaultFactory;

function rebuildMethods(logger) {
  for (let i = 0; i < levels.length - 1; i++) {
    logger[levels[i]] = i < logger.level ? noop : factory(i, logger);
  }
}

function removeMethods(logger) {
  for (let i = 0; i < levels.length - 1; i++) {
    delete logger[levels[i]];
  }
}

function Logger(logName, logLevel) {
  const logger = this || {};

  const defineProperty = Object.defineProperty;

  defineProperty(logger, 'title', {
    get() {
      return logName;
    },
  });

  defineProperty(logger, 'level', {
    get() {
      return logLevel;
    },
    set(lvl) {
      let newLevel = lvl;

      if (typeof newLevel === 'string') {
        newLevel = levels.indexOf(newLevel.toLowerCase());
      }

      if (typeof newLevel === 'number' && newLevel >= 0 && newLevel < levels.length) {
        logLevel = newLevel;
        rebuildMethods(logger);
      } else {
        throw new Error(`Invalid level: ${lvl}`);
      }
    },
  });

  defineProperty(logger, 'levels', {
    get() {
      return levelsInfo;
    },
  });

  logger.use = function (plugin, config) {
    // if (
    //  typeof plugin === 'object' &&
    //  typeof plugin.name === 'string' &&
    //  typeof plugin.factory === 'function'
    // ) {
    if (plugin instanceof Plugin) {
      const pluginName = plugin.name;
      if (!configs[pluginName]) {
        // lazy plugging
        configs[pluginName] = {};
        chain.push(plugin);
        factory = pluginsFactory;
      }

      plugin = pluginName;
    }

    if (typeof plugin !== 'string' || !configs[plugin]) {
      throw new Error(`Invalid plugin: ${plugin}`);
    }

    configs[plugin][logName] = config || {};
    rebuildMethods(logger);
  };

  logger.level = logLevel;
  loggers[logName] = logger;

  return logger;
}

function log(name, level) {
  name = name || '';
  if (typeof name !== 'string') {
    throw new TypeError(`Invalid name: ${name}`);
  }

  return loggers[name] || new Logger(name, level || log.level);
}

Logger.call(log, '', 3);

log.Plugin = Plugin;

log.config = (newLevels, newLevel) => {
  Object.keys(loggers).forEach(logger => removeMethods(loggers[logger]));

  levels = newLevels;
  levelsInfo = levels.slice();

  Object.keys(loggers).forEach((logger) => {
    loggers[logger].level = newLevel;
  });
};

export default log;

Example

const log = require('loglevel');

class Prefixer extends log.Plugin {
  constructor() {
    super('prefixer');

    this.defaults = {
      text: 'default prefix',
    };
    this.prevs = {};
  }

  factory(method, logger, config) {
    return (args) => {
      const timestamp = Date.now();
      const prev = this.prevs[logger.title];
      const delta = prev ? timestamp - prev : 0;
      this.prevs[logger.title] = timestamp;
      args.unshift(
        `[+${delta}ms] ${logger.levels[method].toUpperCase()} (${logger.title}) "${config.text}":`,
      );
    };
  }
}

log.level = 'trace';

(function StackTraceTest() {
  log.trace();
}());

const child = log('child');
child.info('child');

const prefixer = new Prefixer();

child.use(prefixer, { text: 'custom prefix:' });

log.info('root');

child.info('child');

let sum = 0;
for (let i = 0; i < 1000000; i++) {
  sum += Math.log(0.1);
}

log.use('prefixer');

log.info('root');

child.info(sum);

log.config(['verbose', 'trace', 'critical', 'silent'], 'trace');

log.critical('critical');

child.verbose('verbose1');

child.level = 'verbose';
child.verbose('verbose2');

(function StackTraceTest() {
  log.trace();
  child.trace();
}());

Output

C:\Users\u36\Dropbox\kutuluk\logler>node ./examples/example
Trace
    at StackTraceTest (C:\Users\u36\Dropbox\kutuluk\logler\examples\example.js:29:7)
    ...
child
root
[+0ms] INFO (child) "custom prefix:": child
[+0ms] INFO () "default prefix": root
[+27ms] INFO (child) "custom prefix:": -2302585.0930085
[+1ms] CRITICAL () "default prefix": critical
[+1ms] VERBOSE (child) "custom prefix:": verbose2
Trace: [+0ms] TRACE () "default prefix":
    at Function.trace (C:\Users\u36\Dropbox\kutuluk\logler\lib\logger.js:105:14)
    at StackTraceTest (C:\Users\u36\Dropbox\kutuluk\logler\examples\example.js:64:7)
    ...
Trace: [+2ms] TRACE (child) "custom prefix:":
    at Logger.trace (C:\Users\u36\Dropbox\kutuluk\logler\lib\logger.js:105:14)
    at StackTraceTest (C:\Users\u36\Dropbox\kutuluk\logler\examples\example.js:65:9)
    ...

If this fits into the development concept of loglevel, I will make a pull request

@pimterry
Copy link
Owner

pimterry commented Feb 4, 2018

This is really great.

I've got some specific thoughts below, but generally I'm very keen on this, there's a lot of nice improvements here that we can make to iterate on feedback and improve the UX longterm.

There's a couple of places where I'm not totally sold, but most of this sounds superb. The main thing I'm trying to do is ensure that loglevel continues to work out of the box for as many people as possible, even in awkward environments, not just for people keeping up to date with the nicest modern stack.

On your list above:

  • Remove support for legacy browsers.
    • Open to this, but we should be explicit about what that means. I'd be ok with dropping support for browsers with < 1% web traffic right now though. That still leaves IE 11 (~3% of web traffic in Jan 2018), but rules out the rest of IE, and a lot of the other older browsers. I'm pulling stats from here: http://gs.statcounter.com/browser-version-market-share
  • Remove support for persistance levels.
    • I find persistence quite useful. Without it, you can't change the log level used at page startup without rewriting the source of the page itself. I'm open to disabling it by default or something though.
    • We could disable persistence by default, and create a .persist() method instead, which persists the current level.
    • In fact, that could even be a plugin actually (add .persist(), load current level on plugin registratoin), maybe that's the right way to go, and we just need to ensure that's practical...
  • Remove noConflict()
    • Why? There's definitely (sadly) still a meaningful proportion of web development that happens without proper module systems (42% of devs use browserify/webpack, 10% use RequireJS, and 32% don't do any bundling whatsoever, according to the https://www.sitepoint.com/front-end-tooling-trends-2017, this time last year), and if you're using a global then noConflict can be pretty important. Easy to implement, and doesn't hurt anybody else afaict.
    • Generally I'm quite keen to ensure loglevel is useful for as many JS developers as possible, so while there's still a contingent using bare JS & it's easy, it'd be nice to support them. It's useful to let you quickly add nice logging to tiny scripts in HTML pages or into sites like jsbin too, with zero setup required.
  • Сustomizable levels
    • Oooh, nice fix to a common debate, definitely!
    • Curious what the API for this looks like though - how do I define a new level, and map it to a console function?
  • Flat flow of plugins (only one extra line in the stacktrace for any number of plugins)
    • 100%, and we can pull in plugin API improvements en route too
  • log.getLogger('child') => log('child')
    • I don't have strong feelings about this. This is shorter, but getLogger feels clearer. Still, if you do then I'm happy to change it.
  • getter/setter log.level
    • Nice idea, definitely 👍
  • Maybe something else...
    • Built in Flow & TS type definitions would be nice (personally I'd probably just rewrite the source in TS, but up to you)
    • Support for some more console methods (like timing and group)
    • Don't any of that for 2.0.0, just as part of the v2 direction in future.

@kutuluk
Copy link
Contributor Author

kutuluk commented Feb 5, 2018

Speaking of version 2.0, I meant completely abandoning obsolete environments. Backwards compatibility could be saved with the help of the loglevel/legasy submodule. This is a common practice.

But I do not insist, because I understand that loglevel is widely used and breaking back compatibility is undesirable. Therefore, I realized my idea in the form of a package https://github.com/kutuluk/log-n-roll. This is a 900-byte logger with a built-in prefixer. I think this is ideal for modern applications such as SPA and PWA.

But I can say with confidence that all the same thing can be implemented in loglevel. The difference is only in size. So you just need to decide which changes will improve the loglevel and make them.

@karfau
Copy link

karfau commented Feb 7, 2018

Awesome work, and ideas I hope this won't grow into YAL (yet another logger).
I'm happy to contribute to any of this, and able to write TS.
One idea about compatibility:
I haven't totally wrapped my head around it, but maybe this is possible:
Have two kind of builds: one for "modern usage" as @kutuluk called it,
but available under loglevel/modern (since it is easy for newer methods to only require/import that part)
and the fully fledged legacy support still being available under loglevel.

I have't looked at the code base so far, but maybe the compatibility can also be added as a plugin?

@kutuluk
Copy link
Contributor Author

kutuluk commented Feb 8, 2018

Implementing the support of browsers from the Stone Age, you risk yourself stuck in the Stone Age. If this is your choice, then I'm not going to challenge it. But I want to live today and look into tomorrow, instead of hanging in yesterday.

My position is very strong - backward compatibility with browsers no lower than IE 11 (ES5). Ensuring compatibility below is not a library task - it's a task for https://github.com/es-shims/es5-shim.

@pimterry
Copy link
Owner

pimterry commented Feb 8, 2018

backward compatibility with browsers no lower than IE 11 (ES5)

Yep, that sounds good to me. As in my comment above, I think IE 11 is a good & low bar nowadays, and I'm fine with only publishing a 'modern' build that supports IE11+. That's still very inclusive (IE10 is now down to 0.1% of browser usage), and users who want support back even further than that can just stick with loglevel 1.x, which will keep working fine for all sorts of super old browsers, or look at shimming etc if they really need to. A legacy build would work, but I'd rather just point people back to 1.x and keep ongoing dev simple.

The main legacy bit that I would like to retain is UMD, for developers not using webpack and similar, since the stats seem to suggest that's still a significant percentage of developers. That's a cheap, easy and tiny addition though, and doesn't get involved in the core of the code - we just keep the UMD wrapper & noConflict. We could do that in a separate build if we really wanted to, but publishing etc is all quite a bit simpler if there's just a single copy that works everywhere.

@kutuluk
Copy link
Contributor Author

kutuluk commented Feb 8, 2018

I think IE 11 is a good & low bar nowadays, and I'm fine with only publishing a 'modern' build that supports IE11+.

It sounds delicious. My current implementation https://github.com/kutuluk/log-n-roll is fully operational in IE 11. With the help of https://github.com/developit/microbundle three ES5 builds are compiled: UMD (without noConflict), CommonJS and ESM.

My opinion about noConflict(): resolving name conflicts is not a library task, it's a programmer's task. The only thing that is needed from the library is to provide a permanent name, which is occupied in the global scope. One clause in the documentation. Nothing more.

I've already changed my original sketch and now it's almost finished. I propose to take it as a basis, thoroughly think through the API, implement some of the most popular plug-ins (in a simplified form I have already implemented something), test and merge into loglevel under version 2.0 without backwards compatibility. The current version of loglevel is published as loglevel@legacy and maintained separately.

@pimterry
Copy link
Owner

That all sounds great to me. I've opened a v2 branch on this repo, can you PR changes into there? That way we can do a series of PRs to reimplement the core, then update the docs etc, and make various other improvements I'd like around the project, and finally merge & release it all together once it's all ready.

I'm still on the fence about noConflict(). You're right that developers can work around it themselves, but it can be quite awkward, and it's a pretty strong convention. Anyway, we can get started regardless. PR whatever you're happy with to v2 when you're ready, and we can start with that and iterate from there.

@zmoshansky
Copy link

I'd second that it'd be nice to see persisting levels disabled by default (The current behaviour is confusing as we use default_level and if a log level is set, we can't override without explicitly enumerating loggers and calling setLevel.

Alternatively, provide a clear method~

 if (window.localStorage) {
      window.localStorage
      const isLogLevel = R.contains('loglevel');
      for (var key in window.localStorage){
        if (isLogLevel(key)) {
          window.localStorage.removeItem(key);
        }
      }
    }

@sellmic
Copy link

sellmic commented Apr 12, 2018

These ideas all sound great, from my end I don't think there's any must haves in the suggested v2 features. But I'm just evaluating the library at this point, so I'm probably missing a lot of context.

My interest is more on being able to add plugins to the log system in order to have flexibility on sending logs to a server, and it looks like the current version supports that with a few plugins already (or we could write our own).

What's the status of v2, doesn't look there's been much activity on that branch ...

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

No branches or pull requests

5 participants