Built-in debug info #2

@issuehunt has funded $40.00 to this issue. See it on IssueHunt

@sindresorhus I've looked into debug and it looks fairly complicated and opinionated to me. Also, parsing env vars is not a job for event library. Hence, I propose simplified approach: allow user to pass trace hook on a library level and tags on per-emitter level. This way anyone can set up debugging the way they want, and it usually would require just a few lines of setup.

const Emittery = require('emittery');
// global setup
Emittery.setTracer((tag, action, data) => {
  if(!tag) return;
  console.log('emittery', tag.name, action, data);
});
// end of global setup

let emitter1 = new Emittery();
let emitter2 = new Emittery().tag({name:'abc'}); //local setup

emitter1.emit('event'); // produces nothing
emitter2.emit('event'); // produces something like `emittery abc emit {event:event}`

// NOTE It might seem like a good idea to use event emitter instead of hooks, but that would introduce recursion and require somewhat complicated code to handle it.

I've looked into debug and it looks fairly complicated and opinionated to me.

Per my comment, we wouldn't use debug.

Also, parsing env vars is not a job for event library.

Not parsing. Just checking two environment variables.

I like your idea, but it's complimentary to my idea, which is more high-level and for quicker debugging.

Why .tag() instead of an option to new Emittery()? Would debugName be a clearer name?

I think it would be useful to set a tracer on a specific emitter instance. And also be able to set a quick tracer that just prints to stdout:

const Emittery = require('emittery');
// global setup
Emittery.setTracer((tag, action, data) => {
  if(!tag) return;
  console.log('emittery', tag.name, action, data);
});
// end of global setup

let emitter1 = new Emittery();
let emitter2 = new Emittery().tag({name:'abc'}); //local setup

emitter1.emit('event'); // produces nothing
emitter2.emit('event'); // produces something like `emittery abc emit {event:event}`

emitter1.setTracer((action, data) => {
    //
});

// Automatically prints debug info for all instances
Emittery.isDebug = true

// Automatically prints debug info to stdout
emitter1.isDebug = true;

Maybe make new class DebugEmitter that extends Emittery so the default class can stay clean and fast

Why .tag() instead of an option to new Emittery()? Would debugName be a clearer name?

The intent of tag is to provide maximum flexibility without putting too much code(=bugs+dependencies) into library. If we allow passing arbitrary information, with just a couple of lines of code user can customize behavior however he wants. For example

if(process.env.DEBUG_MY_EMITTERY)  { // checking env done by user in one line with full customization
  Emittery.setTracer((tag, action, data) => {
    if(!tag) {
      console.error(new Error('untagged emittery trace'), action, data); // easy detection of places I forgot to tag
      return;
    }
    if(tag.ignore || (tag.ignoreBeforeSomeEvent && !someEventHappened)) return; // quick and dirty filtering of noise in one line
    console.log('emittery', tag.name, action, data); // custom output without complex options built into library
  });
}
let emitter1 = new Emittery().tag({name: 'cde', ignoreBeforeSomeEvent: true});
let emitter2 = new Emittery().tag({name: 'abc'});
let emitter3 = new Emittery().tag({ignore:true});

As of option vs additional method, for me additional method looks clearer, as this functionality is kind of sitting next to emittery, not a part of the main problem it solves. Plus, it's easier to find in code(as all such tag functions on various objects usually exist for debugging purposes). Not really important in this case, but keeping it to in separate method reduces small non-tracing overhead to zero. However, this opinion might not be universal, an option is fine too.

Not parsing. Just checking two environment variables.

But then again, if we provide a good api, adding same checks from user code is a matter of one line, even faster than looking up additional documentation, so why bother including the code into library. Sample above.

Emittery.isDebug = true; // Automatically prints debug info for all instances emitter1.isDebug = true; // Automatically prints debug info to stdout

Not needed with arbitrary type tag. Can be just {ignore:true} tag or whatever user chooses. Sample above.

On the other hand this simplicity allows for even faster approaches when all of this isn't needed:

Emittery.setTracer(console.log.bind(console)); // all one needs to do to just dump all the events into console without bothering about env variables, filters and so on

Apart from simplicity of making same effect using tags, part of reason to only add global level tracer is that it can be done with guaranteed zero overhead in non-tracing(default) mode by just modifying prototype. (@devcat that addresses your concern. If we keep it with tag method and global prototype modification for enabling trace mode, the overhead in non-tracing mode will be exactly zero.)

. (@devcat that addresses your concern. If we keep it with tag method and global prototype modification for enabling trace mode, the overhead in non-tracing mode will be exactly zero.)

It doesn’t address cleanness as default class will now contain strange methods in production

But then again, if we provide a good api, adding same checks from user code is a matter of one line, even faster than looking up additional documentation, so why bother including the code into library.

The use-case for having built-in support for those environment variables is needing to quickly debug the events when you don't directly control the instances. Even if you import the dependency yourself and setup a global tracer, you're not guaranteed to get the same module.

Emittery.setTracer(console.log.bind(console));

And Emittery.isDebug = true; would pretty much do that internally, but also do some nicer formatting.

I don't really see the problem in adding a couple of conveniences. It doesn't affect the performance and makes some common use-cases simpler.

Wouldn’t this just be better design

class Debug extends Emittery {
  constructor({ tag, output = console.log }) {...}

  log(...) { this.output(...) }

  emit(...) {
    this.log(...);
    super.emit(...);
  }

  ...
}

If DEBUG env variable is detected return Debug class instead of Emittery

• No prototype modifications
• Small webpack bundles in production
• No limitation to add more advanced debug features

@sindresorhus has rewarded $36.00 to @sbencoding. See it on IssueHunt

• :moneybag: Total deposit: $40.00
• :tada: Repository reward(0%): $0.00
• :wrench: Service fee(10%): $4.00