peeves & current best practices?

[Tim Oxley]

So, here's part of a totally subjective, non-comprehensive list of frustrations and "best practices" I've been collecting. Some are based on personal experience and others are ideals from the node community that I'm trying to incorporate.

I'm looking for more tips like this. Please share your knowledge or vent your frustrations here.

Stream-like apis
If your code is emitting data/error events but not inheriting from Stream, your module becomes inoperable with other streams, e.g. can't pipe(). This severely reduces your module's flexibility.

Inconsistent sync/async
When functions are sometimes sync and sometimes async lead to hard to use apis, subtle bugs and more error handling wrapper code. Always wrap callbacks in process.nextTick if they're not behind an async operation. Simple.

Prematurely optimising
e.g. choosing to avoid readable, semantic iteration with Array#map/forEach/reduce/filter… instead using for loops 'because they're faster'. 

Unless you're writing a database driver or something similar, focus on writing maintainable, clean code. Optimising your io and designing to scale horizontally rather than tuning individual code paths will reap far more benefit and is more inline with node's focus (io bound tasks over cpu bound tasks).

OO is an implementation detail
Rather than exporting constructors as the primary API of a module, just give me a factory method please.

Fear of dependencies
Some developers exhibit a reluctance to use modules due to the 'overheads' of managing dependencies, which is probably a stigma carried from other non-nodejs environments. It's not nearly as big of an issue in node, so don't hesitate to `npm install` with reckless abandon.

Duplication of effort
Creating new modules instead of improving existing modules dilutes the quality of the module ecosystem. Diversity is great, but only when there’s a meaningful reason for that diversity to exist. "All the other modules for X were broken or buggy", sounds like a good reason to start a new module, but perhaps you're throwing baby out with bathwater… the fix may have been a simple task but instead of 5 functionally similar, sparingly maintained modules, we’ve now got 6, and more surface area for bugs. Nice work. 

Another reason new modules are created is because the consumer didn't appreciate the module's api; this is a trickier issue as people love bikeshedding over this kind of stuff. I've found it's often easiest to just wrap modules with an api you prefer rather than starting from scratch or arguing with the author.

Learn to npm

npm contains a great amount of functionality and information that many people don't seem to know about. For example you should rarely have need to edit your package.json (especially when installing/updating modules) as npm has commands to create and update your package.json for you. 'npm help' is an excellent source of answers to many questions. There's plenty to learn about npm, please share any tricks.

npm scripts vs readme
If you encapsulate all the knowledge about how to build/boot/test your app into your package.json scripts, that's less information you need to put into the readme and keeps things consistent. For example, if you setup a test script, I only need to know to run `npm test`, rather than hope you've included information about how to run your tests. If you change your test tool, or change the arguments, I don't even need to know since that information is embedded in the package.json.

Tagging releases
If you don't tag releases, it's a pain to find the commit that corresponds to a release. Use npm version to generate tags.

Deep abstractions

Having many layers and long breadcrumb trails makes understanding and debugging your module difficult, raising the barrier to participation (and maintainability). Complexity is often presented as 'architecture'. Simplicity is key. If your system is becoming complicated, it's probably doing too much or you're going about it in the wrong way.

Refrain from building/using all-in-one frameworks
This is not in keeping with the nodejs aesthetic. Node isn't Rails. Having code coupled to a framework creates duplication of work, fragmentation and interoperability. This is bad. The trend towards modularisation is one of the best things node has got going for it over other environments, don't ruin it by creating silos.

MVC
Usually overkill. If your app is modular enough, you likely won't need much/any of it. MVC everywhere makes it more difficult to isolate features and break them into decoupled components, as well as encouraging some utterly ridiculous solutions (when all you have is a hammer…). Frameworks that dictate application architecture should be avoided as architecture should be determined by the problem at hand not a framework. Moving away from MVC everywhere also gives you the ability to experiment with different patterns and evolve your architecture toolset. You'll be surprised at how simple some problems are if you don't contort everything to fit into the MVC mould.

Supply usage examples in modules
Most of TJ's and Substack's modules include working examples demonstrating various use cases. Showing how to use the module is far, far more important than narratives and lengthy api documentation in a readme. Code is a concrete explanation and usually uses less words (if not perhaps your code needs refactoring).

Supply usage examples upfront in readmes
Please put a usage example at the top of your readme.

Write tests

Tests help other developers have confidence they haven't accidentally broken anything when contributing to your project. Tests can also serve as usage examples in lieu of comprehensive examples/documentation.

What else? Teach me of your ways.

This part of some research for a presentation I'm putting together on a similar topic.

[Mark Hahn]

That's pretty awesome.  There's a couple of items I'd like to discuss, the first one being ...

> Always wrap callbacks in process.nextTick if they're not behind an async operation. Simple.

This doesn't seem necessary to me and it seems inefficient and dangerous.  I make sure to write in a style that will work whether the callee is sync or async.  Your scheme will break if the callee ever adds some async.  Writing in a style that isn't affected by sync/async has these advantages ...

1) I don't have to know whether the callee is sync or async.  I'm lazy.

2) As I said above, I don't have to worry if it changes.

3) It makes it easier to add my own async to stuff.  I can just split the code with the editor.

4) Also as I said above it is efficient.

--
Job Board: http://jobs.nodejs.org/
Posting guidelines: https://github.com/joyent/node/wiki/Mailing-List-Posting-Guidelines
You received this message because you are subscribed to the Google
Groups "nodejs" group.
To post to this group, send email to nod...@googlegroups.com
To unsubscribe from this group, send email to
nodejs+un...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/nodejs?hl=en?hl=en

[Tim Oxley]

> Always wrap callbacks in process.nextTick if they're not behind an async operation. Simple.

Oh, of course… to clarify:

If a module is possibly going to execute an async operation, ensure any otherwise synchronous callbacks are inside in process.nextTick. i.e. If it's sync it should always be sync. if it's async, it should always be async.

[Mark Hahn]

OK, I realize now that all your points were about writing good modules.  I took your point from the view of the module user, not the author.  I really should read more carefully.

[Tim Caswell]

Great writeup! Most these points are right on. I take issue with a
couple of them and have a different opinion. (That's right, this is
my opinion, not saying you're wrong)

> ## Inconsistent sync/async

>
> When functions are sometimes sync and sometimes async lead to hard to
> use apis, subtle bugs and more error handling wrapper code. Always wrap
> callbacks in process.nextTick if they're not behind an async operation.
> Simple.

I used to preach this. It does make it safer for users of your
library to not have to worry. But I've also been severely bitten by
the performance and race-condition implications of this. Waiting till
nextTick to call a callback when you know the answer right away is
extra effort. I don't think a blanket rule that you should always do
this is right. I've seen many cases where it's better and simpler to
just call right away.


> ## Fear of dependencies

>
> Some developers exhibit a reluctance to use modules due to the
> 'overheads' of managing dependencies, which is probably a stigma carried
> from other non-nodejs environments. It's not nearly as big of an issue in
> node, so don't hesitate to `npm install` with reckless abandon.

When developing a library please be aware of the cost of depending on
a library. Especially if that library in turn depends on 10 other
libraries. Your library will be just one of the many in my app. I've
seen many cases where pulling in just one library causes my `npm ls`
output to be completely useless. I'm only using 3 libraries directly,
but I have several screen-fulls of stuff. And stuff I know I'm not
using like 5 different kinds of option parsers, Ansi color libraries,
logging frameworks, and sometimes even testing frameworks.

Please be aware of the person using your library. Every dependency
you have will be yet another dependency for every person using your
library. I shouldn't have to install 5MB of javascript code in
dependencies just to use your simple library.

[Tim Oxley]

Great writeup!  Most these points are right on.

Thanks Tim! Positive feedback is welcome. 

 

But I've also been severely bitten by 
the performance and race-condition implications of this.

...

I've seen many cases where it's better and simpler to 
just call right away.  

Can you give me an example?

 I've seen many cases where pulling in just one library causes my `npm ls` 
output to be completely useless.

I've never found `npm ls` to be particularly useful anyway, I'm generally only concerned by the packages defined in my package.json. What are you using `npm ls` for?

 

 And stuff I know I'm not using like 5 different kinds of option parsers, Ansi color libraries, 
logging frameworks, and sometimes even testing frameworks. 

Obviously testing frameworks should have been put in dev dependencies, that's an obvious mistake, but how would you suggest people author libraries that offer a commandline tool that supplies pretty output, without clogging up the deps and without reinventing the wheel? I know that was just an example, but I don't see a way of avoiding deps for functionality you may not be using without going full substack.

 

I shouldn't have to install 5MB of javascript code in 
dependencies just to use your simple library. 

 

Probably true especially regarding code complexity, but disk space is cheap. Though, the time to install all that stuff is a real bummer. If `npm install` was faster, would this be less of an issue?

[Bradley Meck]

 

Probably true especially regarding code complexity, but disk space is cheap. Though, the time to install all that stuff is a real bummer. If `npm install` was faster, would this be less of an issue?

Disk is cheap, but if you actually do end up loading all the libraries at once you can see serious memory bloat for what could be a small one off script (have seen 20MB of bloat on some user's apps). 

[mscdex]

On Oct 11, 2:24 am, Tim Oxley <sec...@gmail.com> wrote:
> Probably true especially regarding code complexity, but disk space is
> cheap. Though, the time to install all that stuff is a real bummer. If `npm
> install` was faster, would this be less of an issue?

FWIW the latest version of node-gyp has the ability to specify how
many cpus/cores to use when compiling addons, so that should make it
faster in those cases.

[Axel Kittenberger]

> Inconsistent sync/async
> When functions are sometimes sync and sometimes async lead to hard to use
> apis, subtle bugs and more error handling wrapper code. Always wrap
> callbacks in process.nextTick if they're not behind an async operation.
> Simple.

As others have written, if you have to have to do this,then your async
handling is cranked up. Caches aren't there for nothing. If the callee
depends on a particular order that it is in error not you, and I'd
rather see errors fixed than leaving them in and not raising'em. I
won't say, use one of the 1000ooo.... (a)sync libraries, but if the
callee would use one of them (like my favorite streamline) the
"problem" won't be one, and that one for example will get good
performance using trampolining technology. This is only sanely
available to you if you are the application developer, if you develop
a library, just take care.

[Adam Crabtree]

I think what the OP means, is that library authors should wrap *synchronous* callback calls in process.nextTick, which is a best practice.

If you're saying (possibly) that it's not, then I disagree. Library code should be predictable, not synchronous in some cases and asynchronous in others. process.nextTick is not slower, it adds only the minimal deprioritization necessary to execute your code asynchronously (i.e. consistently). This does not undermine any cache performance as nextTick executes on the order of <1ms, significantly faster than any I/O.

Cheers,

Adam Crabtree

--
Job Board: http://jobs.nodejs.org/
Posting guidelines: https://github.com/joyent/node/wiki/Mailing-List-Posting-Guidelines
You received this message because you are subscribed to the Google
Groups "nodejs" group.
To post to this group, send email to nod...@googlegroups.com
To unsubscribe from this group, send email to
nodejs+un...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/nodejs?hl=en?hl=en

--
Better a little with righteousness
       than much gain with injustice.
Proverbs 16:8

[Tim Oxley]

I think what the OP means, is that library authors should wrap *synchronous* callback calls in process.nextTick

…yep, I mean wrap synchronous callbacks if there's any possibility the function might execute asynchronously… though Tim Caswell suggests above that this may not be best practice, and I am eagerly awaiting further elaboration.

-Tim

[Tim Caswell]

On Thu, Oct 11, 2012 at 2:19 PM, Tim Oxley <sec...@gmail.com> wrote:
>> I think what the OP means, is that library authors should wrap

>> *synchronous* callback calls in process.nextTick
>
>
> …yep, I mean wrap synchronous callbacks if there's any possibility the
> function might execute asynchronously… though Tim Caswell suggests above
> that this may not be best practice, and I am eagerly awaiting further
> elaboration.

I'm merely saying that there are situations where the performance
matters more than preventing users of your code from shooting
themselves in the foot and a blanket rule to *always* be consistent is
a bit much. This is less of a problem now that nextTick is a lot
faster than it used to be.

On the whole, I agree that being consistent does make things easier
for people using your library. I'm just unsure if it's worth the
cost. That is all. I'd rather see client patterns that are immune to
callbacks being called before the function returns sometimes.

[Axel Kittenberger]

> I'd rather see client patterns that are immune to callbacks being called before the function returns sometimes.

Ditto!

We should encourage people to write callers that are good, rather than
libraries that deliberately waste performance and tell the callers
"its alright you wrote bad code, they have to put in a
process.nextTick anyway". And < 1ms can be a lot in some areas.

Document your function accordingly, if it guarantees a particular
callback/return order or not. In many situations, standard is,
callback immediately if you have all what is needed for the callback.
If the caller fucks up, that one should be fixed, instead of the
callee.

Or in other words, cure the problem, not the symptom.

[Adam Crabtree]

It's a best practice because it helps those unfamiliar with the reasoning to keep from shooting themselves or their users in the foot. There are several ways that this may affect you, but a quick summary can be found here:

http://howtonode.org/understanding-process-next-tick

How slow is process.nextTick? A quick benchmark reveals it's not just <1ms, but in fact is roughly 1µs (0.001ms for the lazy):

var i = 0, sum = 0

;(function foo() {

  var t = process.hrtime()

  process.nextTick(function() {

    sum += process.hrtime(t)[1]

    if(++i<10000000) return foo()

    console.log('Average time: ', sum/i)

  })

})()

That being said, there are always exceptions to the rule, and if you understand the tradeoffs and have a need to shave off µs, then go for it. Chances are though, for the other 99.9% it's a micro-optimization (no pun intended ;P). Again, this requires a special set of circumstances to be an issue, but when it is, discovering that the cause was a cache hit and a synchronous call to callback can be frustrating.

Cheers,
Adam Crabtree

--
Job Board: http://jobs.nodejs.org/
Posting guidelines: https://github.com/joyent/node/wiki/Mailing-List-Posting-Guidelines
You received this message because you are subscribed to the Google
Groups "nodejs" group.
To post to this group, send email to nod...@googlegroups.com
To unsubscribe from this group, send email to
nodejs+un...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/nodejs?hl=en?hl=en

[Ted Young]

All good points!  I would add:

When in doubt, use callbacks

One gotcha that only occurs once you are well into a project is having a synchronous operation that returns a value turn into an asynchronous operation that requires a callback or event notification.  This can create an untold amount of refactoring, as any code that depended on this operation may have to change it's api to use a callback, and so on up the chain.  That can add major drag to your development.  So, when in doubt, use a callback as your api.

Basic CI is too easy, there's no excuse not to use it

You can get your module tests up and running on travis-ci in like, 5 minutes.  If it takes longer than that, there's probably some kind of deployment issue you need to deal with.  So at minimum you should be using travis-ci or something similar for all production-ready modules.  I feel a lot better about modules and services if I see them running on CI successfully.

Ted

[Jimb Esser]

Though process.nextTick() *itself* is fast, delaying calling the callback until it gets through that queue can have large performance implications, for example, in our case, we may have a tick of our physics simulation queued up (which could take hundreds of ms), and if some logic has to go through a few process.nextTicks, all interspersed with some other expensive operations in between, this kind of API can lend itself to some poorly performing side effects.

That being said, I do agree that it's generally "best practice" to do this, but it's good to be aware that it's not always the best for performance (in some of our own APIs, where we set them up to always call the callbacks asynchronously, we have needed to add short-cuts in a couple of cases where it had a significant impact on latency).

[Bradley Meck]

Just beware of Travis CI failing for situations that are beyond your control. Missing C libraries, OS issues, and external resource needs can all be problematic. Also I have noted at least in the past sometimes travis fails to provision VMs appropriately. Running tests in your environment is the best way to test a module.

[Ted Young]

The hosted version of travis only uses 32-bit Ubuntu, which is a reasonable baseline target for an open source project.  If your production environment is different enough, you would need to run your own CI and not use the hosted version of travis.  

I guess my point is "travis-ci should represent the minimal reasonable testing effort" for an open source project you expect others to depend on.  If it's not a simple, automated process to get your project installed on something as mainstream as ubuntu, you should fix that.

Ted

On Oct 11, 2012, at 3:49 PM, Bradley Meck <bradle...@gmail.com> wrote:

Just beware of Travis CI failing for situations that are beyond your control. Missing C libraries, OS issues, and external resource needs can all be problematic. Also I have noted at least in the past sometimes travis fails to provision VMs appropriately. Running tests in your environment is the best way to test a module.

[Tim Oxley]

Great, I agree, especially on the basic CI since travis makes it so simple. 

It amazes me how long some projects sit with failing travis tests… when failure is the norm it reduces the benefit of having CI in the first place. Time constraints are probably the issue here, though perhaps the solution is "if you can't get your changes passing, push them onto a separate branch and restore the main branch to a passing state". Again, the primary benefit here is inspiring confidence in contributors and module consumers.

- Tim

[Tim Oxley]

Yep, the idea of best practices is "do this unless you have a good reason not to", which doesn't mean it's a blanket rule that must never be broken. A guideline, not a rule.

The main issue with inconsistent sync/async functions is the behaviour has low discoverability unless it's documented (unlikely), you read the source, or you get gotcha'd by it.

-Tim

[Dominic Tarr]

I was worried for a second that this post was gonna be about punctuation.

Pleasantly Surprised!

The hardest part is the bit about NIH. This isn't really something we understand properly yet. It can be a struggle just to find other modules that do the think you want. Sometimes you've written a module before you even discover that other solutions exist.

If you do find someone has a module that is close to what you need,

but not quite, in some important way, then you need to communicate with them. The best way to do this is on IRC. Unfortunately not everyone uses IRC.

Please use IRC.

Code is a personal thing, and it's important to try and understand the VIBE the author is going for. Issues aren't really a way to communicate vibe.

If someone is posting issues, or telling you about stuff in irc, please listen to them. Even if they are annoying. They will probably improve the usability of your module quite a bit. 

To really understand this though, I think we need anthropologists to live with hackers, and write a whole book about it.

[Tristan Slominski]

On Thursday, October 11, 2012 1:24:12 AM UTC-5, Tim Oxley wrote:

Great writeup!  Most these points are right on.

Thanks Tim! Positive feedback is welcome. 

 

But I've also been severely bitten by 
the performance and race-condition implications of this.

...
I've seen many cases where it's better and simpler to 
just call right away.  

Can you give me an example?

 Tim, we ended up with some nextTick benchmarks a while back. Here is my latest output for node v0.8.11:

loopsPerSecond: 431034482.8, nextTicksPerSecond: 1616854.4, ratio: 266.6x times faster

loopsPerSecond: 434782608.7, nextTicksPerSecond: 1580382.8, ratio: 275.1x times faster

loopsPerSecond: 432900432.9, nextTicksPerSecond: 1582450.1, ratio: 273.6x times faster

loopsPerSecond: 436681222.7, nextTicksPerSecond: 1591237.3, ratio: 274.4x times faster

benchmark here: https://gist.github.com/1512111 (forked from https://gist.github.com/1511972)

discussion here: https://groups.google.com/forum/#!topic/nodejs/PErl27A4e-A/discussion

[Rick Waldron]

On Friday, October 12, 2012 at 6:15 AM, Dominic Tarr wrote:

I was worried for a second that this post was gonna be about punctuation.

Pleasantly Surprised!

The hardest part is the bit about NIH. This isn't really something we understand properly yet. It can be a struggle just to find other modules that do the think you want. Sometimes you've written a module before you even discover that other solutions exist.

If you do find someone has a module that is close to what you need,

but not quite, in some important way, then you need to communicate with them. The best way to do this is on IRC. Unfortunately not everyone uses IRC.

Please use IRC.

+9001 

[Dominic Tarr]

It's really about collaboration. The answer to the problem "too many modules" isn't Write Less Modules, it's Collaborate More!

the ability to collaborate is a soft human skill, but a skill that you can develop.

[Jake Verbaten]

we should also build tools to enable collaboration. I'm not sure what those would look like.

Solving the "its hard to discover modules" problem is a lot easier in comparison.

[Bradley Meck]

My problem is not with using Travis, it is a warning. Travis does sometimes fail, and we have had situations where commits cause tests to "fail" according to travis while all local tests run on all machines we test. The cause of this has generally been the problems mentioned.

[Tim Oxley]

Excellent. This is a far more positive angle. Point taken.

[Dominic Tarr]

Jake, it is tempting to think that the there is a technical solution to every social problem.

In a way, git, and github is like this. 

But there is so much more that, as coders, we are only starting to discover. 

Musicians probably have a lot of experience in this area.

[Jake Verbaten]

There is a technical solution to NIH.

I have a new project scaffolding tool which I give a project name & description. 

I'm sure I can get it to auto generate a README saying "I SUFFER FROM NIH" if it finds a module that already matches the description.

[Axel Kittenberger]

Wasn't the motto and the whole idea about Node.JS: "Microseconds matter"?