micromatch / nanomatch

Fast, minimal glob matcher for node.js. Similar to micromatch, minimatch and multimatch, but without support for extended globs (extglobs), posix brackets or braces, and with complete Bash 4.3 wildcard support: ("*", "**", and "?").
https://github.com/micromatch
MIT License
96 stars 20 forks source link

Switch from `debug`? #2

Closed phated closed 8 years ago

phated commented 8 years ago

I'm not a huge fan of the debug module. It requires localStorage in the browser so if you try to browserify/webpack it for usage in a chrome extension/app, the code throws. Have you thought about using https://www.npmjs.com/package/debuglog instead?

jonschlinkert commented 8 years ago

requires localStorage in the browser

good to know. Want to do a pr? Or I can swap it the next time I make changes. thanks

edit: same on the other two related issues, thanks for letting me know

jonschlinkert commented 8 years ago

also fwiw, the main reason I'm using debug (or debuglog) is that it seems to have gotten more difficult to figure out which modules is actually being called by node. Even though flattening was supposed to make things easier, that hasn't been my own experience. If you have other suggestions for this maybe we don't even need debug.

phated commented 8 years ago

I'm poking around on some other stuff today. I'll circle around at some point

jonschlinkert commented 8 years ago

no prob. I'm adding unit tests to this right now anyway, I went ahead and made this change already

jonschlinkert commented 8 years ago

actually I'm just going to remove debug, since this would defeat the whole purpose and advantage of it, which is that you can just do DEBUG=* and it works for any and all modules in the dependency tree that use debug. I'd have to document the API/usage differences and it would need to be used by every lib in the tree for it to be useful, but the debuglog api doesn't have parity and isn't as useful. Also, fwiw, there a ton of libs using debug, I don't see how not having it here would have any appreciable change (I have to imagine there are several libs in the vinyl/gulp trees that use debug, given how popular and useful it is).

anyway, fwiw, this seems more like (yet another) bug in webpack / browserify. It seems like too many people are expecting node.js libraries to support those tools, but I'm having a hard time understanding why that makes any sense at all. I remember when I first started using node, and it seemed like all of the libs were written to be used in the browser or with jquery. Then code started getting cleaner when node conventions became more popular. It seems like node libs should be idiomatic for node usage, and those libraries should find a way to work with that, either by fixing the bug, or creating a browserify or webpack plugin/filter that removes the browser version of debug.

just my 2c

edit: if you think about it, node.js will turn into even more of a cluster*%$# if people start writing their code around webpack and browserify. I really hope that doesn't happen.

phated commented 8 years ago
  1. It's not a problem in any tool, but a problem in the runtime (chrome apps); however, we aren't in a position to change that but we are in a position to be wary of our dependencies.
  2. debug is probably the worst logging tool in the ecosystem. It doesn't have log levels, pluggable output, etc. I'm guessing people just use it because it was one of the first things out when many joined the ecosystem.
  3. People are already writing code around webpack and browserify (see the react ecosystem) and publishing to npm.
jonschlinkert commented 8 years ago

well, it goes without saying that most good developers have strong opinions, and for good reason lol.

we are in a position to be wary of our dependencies

Every maintainer is in this position. We might continue to disagree on this debug point, but I'm clearly making a concession by removing debug to accommodate your request, and the point you're implying is not lost on me.

debug is probably the worst logging tool in the ecosystem.

I imagine it is, given that it's not promoted as an alternative for a logging tool. Honestly - aside from the browserify/webpack issues - maybe your dislike for the module is based on a misunderstanding of it's purpose. My interpretation is that it's a debugging tool, and it's used for something completely different than logging. Levels and such don't really seem to make any sense for debug (especially given that you can create namespaces using whatever custom code you want. I've actually created custom logging levels for an implementation of debug in the past). But I wouldn't use it as a replacement for a good logging tool.

People are already writing code around webpack and browserify (see the react ecosystem) and publishing to npm.

Which makes complete sense if those libs are written for webpack, browersify or react and they prefix the module names with webpack-, browserify- and react-. If they don't then they are cluttering up the ecosystem with bait-and-switch tools that are not generalized for node.js usage. What's the difference between that and publishing gulp plugins without the word gulp- in the name? Or encouraging people to publish their modules as grunt plugins?

Anyway, I think this is getting off of the point of this issue, which I've already resolved. Also, it might be good to know that there are prs open on debug that fix the browser issues.

phated commented 8 years ago

@jonschlinkert just wanted to say that I wasn't trying to be adversarial. Thanks for addressing this and for everything you do. Have a great weekend!

jonschlinkert commented 8 years ago

@phated thank you for saying that. I appreciate you too, I think you're a great maintainer and I know how much you care about the quality of your projects. have a great weekend as well!

tunnckoCore commented 8 years ago

not wanting to continue the discussion, it is perfectly clear for me. just wanting to point that many "gulp plugins" are just streams and are incorrectly prefixed with gulp-.

j2c, hf : ) no bad feelings

Download commented 8 years ago

@phated @jonschlinkert Ok this is a very interesting discussion! I actually built a logger that offers more functionality then debug, but is smaller at the same time (~1kB minified and gzipped). It's called ulog if you are interested.

this would defeat the whole purpose and advantage of it, which is that you can just do DEBUG=* and it works for any and all modules in the dependency tree that use debug.

Exactly! Which is why ulog has adopted this exact functionality and brought it to the browser. On Node, you can set LOG=warn or whatever to set a global log level, or you can set individual modules to level debug with the old and trusted DEBUG=my-module syntax. On the browser, you can specify ?log=info or ?debug=my-module, or even ?log=info&debug=my-module in the querystring. I've been using it for a while and it works really well if I do say so myself :)

Download commented 8 years ago

Oh and one more thing... I've found a convenient pattern for using ulog (or debug for that matter) without forcing it down the library user's throat. It uses ulog only when installed separately by the user of the library. In the library itself, you install ulog as a dev dependency. Then you import it like this:

var log = {name: 'my-module'};
try {
  log = require('ulog')(log.name);
}
catch(e) {
  function nop(){}
  log = Object.assign(log, console, {debug:nop, log:nop});
}

This way, you get a logger object with a name and debug and log methods that you can just use. If ulog is installed, the default log level will ensure your log and debug messages are not printed. If ulog is not installed, log and debug will be no-ops, but info, warn and error are still available.

I'd be happy to cook up a PR for you to add ulog to this project if you'd like.