Node.js Best Practices | Hacker News Reader

Node.js Best Practices

joyent.com

220 pointsfinspin63 comments

Notice something not on the list of best practices: documentation. While Node itself has excellent docs (for the most part), the Node community is terrible about documentation. If you're lucky you'll get a single README.md file with the basics covered.

Pick any category of module and there's a good chance the most popular modules have little documentation; certainly nothing close to comprehensive.

Do they really need more than a single README.md file? Many of those modules are so small that anything more than a single README of documentation would be excessively verbose.

I've never ran into the problem of feeling a module was poorly documented. The especially complex modules usually have more extensive documentation on their website. I'd say there are cases that are terrible or useful documentation for sure, but in general most modules are very good about it from my experience.

I agree with this. The surface area of most modules is small enough to be covered in a README.md, which is displayed prominently on npmjs.org, github.com and a copy of it is in your ./node_modules folder.

As said earlier, the core Node.js framework has great documentation, as do other frameworks such as Mongoose and Express.

I agree with everything you say except for Mongoose.

Mongoose lacks a lot of documentation on the fringe use cases. However, the beauty of open source is that you can just read the code, which, in my experience, is the most reliable documentation.

I want to write a simple node application that fetches an http document and write it to the disk.

The only requirement is for the application to print out in plain English if there is one of the following error:

* the document doesn't exist

* there is a connection error during download

* the local file couldn't be opened

* an error occured during writing.

For any other error(out of memory,...) the program can crash.

How do I do that in node? I don't know.

The request package only tells me the callback's first parameter is "An error when applicable (usually from http.ClientRequest object)". The fs package only tells me "Event: 'error': Emitted if there was an error when writing or piping data."

I only picked error reporting as an example so I could showcase the problem of lack of documentation on the most basic APIs (http get and writing file). By the way the Error Handling article [0] on Joyent's website is absolutely wonderful.

Call me old fashioned but how can I use an API that doesn't tell me which function to call, what a function accepts as parameters, what it returns or how it signals error?

[0] https://www.joyent.com/developers/node/design/errors

HTTP has standard error codes that tell you why a file didn't download (ex. 404 means it doesn't exist). File systems behave similarly (ex. ENOENT). None of this is language specific and therefore do not belong in Node.js documentation (except perhaps to say that these Node.js libraries also adhere to the same standards everyone else does but this is obvious the moment one of these error objects is printed or inspected).

>but this is obvious the moment one of these error objects is printed or inspected

The point of documentation is that you don't have to run the code to see what the code does.

Unlike e.g. in Python, in Node people aren't so eager to use (partially) autogenerated docs that document functions and classes. Sometimes this is an advantage (because handwritten docs are usually better), sometimes not (when the docs no longer match the actual API).

As to whether Node.js projects in general are better documented or worse than the average project in other languages... I dunno, that seems kind of anecdotal. Basic building blocks like `async`, `underscore`, `commander` etc. all have great documentation.

Your average node module is 50-300 lines of code. A readme is just fine to document the 2 or 3 functions that are usually exposed. Beyond that, reading the source is not that onerous. Compare that to your typical doxygen-generated mega-doc, and I think it's a feature, not a bug.

This. Especially the number of simple examples is lacking. Compare the documentation for node's 'request' with the Python docs for 'requests':

https://github.com/mikeal/request

http://docs.python-requests.org/en/latest/

And 'request' is one of the better documented Node modules! Most authors don't bother with any documentation at all and take an 'if you're too dumb to read the source you shouldn't be using my package' attitude.

Of course there are exceptions, async is pretty nicely documented for example.

Related question, is there anywhere in the node 'request' docs that describes the structure of the 'response' object?

The lack of simple examples is far more frustrating than docs personally.

making api docs is a pain in the butt. any recommendations on "api to html docs" tools?

Maybe take a look at OxDoc: http://oxjs.org/#examples/oxdoc_tutorial

Awesome. Thanks!

Just a few comments:

> if (!(this instanceof MyClass)) return new MyClass();

If you really want, just throw an exception and kill the program at compile time. Catch programming errors in testing and not do some magic to 'autocorrect' code.

> var localFile = fs.createWriteStream('localFile.tmp');

Always catch 'error's in stream objects. Otherwise, it might thrown an exception at runtime.

localFile.on('error', /* do something */)

Coding style: In most cases if you write you code properly, you don't need to nest more than 3-4 levels. If it gets deeper split it out into separate functions. Otherwise, it's a perfect job for async.series.

Agree about the instanceof trick. I hate that pattern. I'm not sure if it's done to avoid using "new" or because people legitimately make the mistake.

It's to avoid using "new".

> It's to avoid using "new".

yeah,well one is still using new inside the constructor.

it's pretty obvious to me that :

1/ one should read the docs before using an api

2/ capitalized functions are meant to be used as constructors.

3/ If you dont like this pattern,write builders and make them obvious. like Object.create()

To me when i design js APIs, I have 2 kind of "templates" :

the jQuery "$(o).after(b).get(0)"/underscore "_(object).method().value()" monadic api style that wraps types (like promises)

The way the DOM is built,more java-ish with builders everywhere ( document.createElement ... ).

So I hardly use new anymore.

I get that people might make mistake or like Python style OO;but still, I think forcing the "new" in unecessary.

Cant you just "return this;" to avoid using "new"?

> Cant you just "return this;" to avoid using "new"?

THIS will be the global scope if a function meant to be a constructor is called without new.

Try that in your console

   function Foo(){this.foo = "bar";return this}

then do :

   Foo() ;

it will return window;

so the answer is no.

The answer is yes if you do:

    Foo.call({});

That's a cool trick, though 'instanceof' won't work in this case.

Yup. The proper way to emulate new is

    Foo.call(Object.create(Foo.prototype))

Douglas Crockford, is that you? Oh the lengths people will go to avoid using 'new'...

I wonder if any linters out there warn when they see something like:

  something = SomeCapitalizedFunction()

Because forgetting to use 'new' is really the only thing I could think of that makes this pattern dangerous.

Yes, jslint and jshint both do, by default, iirc. They'll also warn on the opposite (using 'new' with a function that doesn't start with an uppercase letter).

Ah that's fantastic, i've been using them for a while and never noticed.

why do people want to avoid using new?

If you accidentally forget to use 'new' when you're supposed to, the "constructor" method will change the global object instead of a new object, it can cause strange behavior that is very hard to trace.

Ah, I see. I guess I've never forgotten to use new. That does sound incredibly annoying to debug.

It's not about "making a mistake", it's about cases where you'd want to create objects but using `new` would be awkward. E.g. `myListOfRawData.map(MyConstructor)` when you unmarshal data. Sure, you could create an anonymous function that calls new. But it's easier to create higher level functions and tools when the new is optional.

I'm coding a fairly large application in Node and have never heard of EventEmitter or Streams. Does that mean I'm doing something wrong? The impression I get from the article is that it's such a fundamental patterns that every serious application should use it.

When you say "fairly large" do you mean "a large web app" on top of Express? If so don't worry about it.

If you are doing something less "frameworked". Something like custom (text/audio/video) (processing/compression/analysis). You would probably be reinventing Streams if you're not explicitly using them. Meanwhile, apart from implementing Streams, I can't think of a good usecase for EventEmmiter that isn't already in a framework like Express or Socket.io (Someone please enlighten me).

They are the two most common node abstractions. You can get by without them but it's pretty important to know about them.

Streams are the more important one. Once you embrace streams in your applications a lot of things become easier. For example here's a small script I wrote at work to process some csvs and do stuff with them

https://gist.github.com/jb55/0ea6aba86e269f1e526b

Needless to say before I knew about streams that program was twice as large and twice as buggy. Also check out substack's stream handbook to learn more, they are really handy.

"small"

It's 28 SLoC. That's pretty small.

Can you show the 'before' for comparison?

Yes. If you have never heard of either of these I would question whether you are sure you are even writing an application in node.

I'm personally not a huge fan of the eventemitter for cases where an event is only ever emitted once (an sql query is done or similar).

For 2.0 of Sequelize we've moved almost the entire codebase to promises and will encourage users to interact with Sequelize with promises.

Promises and Events are not mutually exclusive. I can imagine a lot of scenarios where both would make sense. But as you said, it doesn't feel right to overuse the EventEmitter mechanism where it's not really needed.

Not that long ago, anonymous functions and closures were the height of fashion in every language.

Now the recommendation is to name your functions and avoid closures. This brings us right back to what C programmers have been doing with function pointers since forever. Not that this is such a bad thing.

When they say 'avoiding closures', how does that relate to functions in your module? Your module is often exported as a function, so are they suggesting that every function be exported? I suspect I'm not understanding the logic behind 'stack based'. Why is it better to have your functions not contain other functions (or is it not be contained?)

It appears they are warning about using closures everywhere to avoid memory leaks.

> are they suggesting that every function be exported?

No. Just export the functions that are used outside of the module. Other functions can be private to the module.

> Why is it better to have your functions not contain other functions (or is it not be contained?)

1. It can make the code clearer

2. It eliminates one source of memory leaks

3. It allows the V8 runtime to optimize more of your code. There is a function length limit (including comments) before V8 will no longer optimize a function.

module.exports = function fOne() { function fTwo() {

    }

    do something...
    fTwo();

}

Can become:

function fTwo() {

}

module.exports = function fOne() { do something... fTwo(); }

v8 (and all other JS engines I tested) are kind of stupid when it comes to garbage collection. You can see a detailed explanation of the problem in this blog post:

http://point.davidglasser.net/2013/06/27/surprising-javascri...

Anyone have more insight into the statement "Avoid closures"? Or rather, an alternative to having private functions?

I interpret 'Avoid closures' as just don't define a function within your function. Private functions are simply functions that haven't been exported and aren't really closures (if we're talking about using a module pattern, at least).

Excessive closures can lead to memory leaks. Anonymous closures are a PITA when debugging.

Closures are fine, but avoid seagulls:

function() {

  function() {

     function() {

        function() {

        }

      }

   } 

}

Seagulls?

First I've heard the word used in this context, but I assume it refers to the appearance of the braces forming a 'V' pattern of what look like seagulls.

"Seagulls" amazing! I'm used to calling them pyramids of doom, or laser guns.

We call it arrow code. The arrow points down a path of deep despair and frustration.

Looks a bit less like seagulls if you do "});" instead of plain "}" though

I think this is going to catch on.

Deeply nested closures are harder to inline and optimize. If you want fast code then the easiest way to accomplish that is with explicitly spelling out everything in the prototype instead of generating closures.

They mentioned memory leaks as their reasoning. Run the following program in any browser and you will see the memory usage balloon out of control:

    var theThing = null;

    function replaceThing(){
      var oldThing = theThing;
      function unused(){ return oldThing }
      theThing = {
        longStr: new Array(1000000).join('*'),
        someMethod: function(){ }
      };
    }

    setInterval(replaceThing, 1000);

V8 (and all other engines I tested) will save the oldThing variable in someMethod's lexical environment record, causing each Thing to keep a reference to the previous Thing, preventing it from being garbage collected. This is despite the fact that the old thing is actually unreachable - someMethod never uses the oldThing variable.

For a more detailed explanation, check out the post that I lifted this example from: http://point.davidglasser.net/2013/06/27/surprising-javascri...

Example of Google's Closure Compiler eliminating the memory leak:

    var theThing = null; 

    function replaceThing() {
      theThing = {
        longStr: Array(1E6).join("*"),
        someMethod: function() { }
      }; 
    } 

    setInterval(replaceThing, 1E3);

http://closure-compiler.appspot.com/home

Very useful!

0. Avoid using it, unless truly necessary.

I have one word: CoffeeScript.

pffff, that's not even a word. It's two words stuck together.

Typewriter..?

Hairsplitter.

Recently I prefer using ES6, and transpiling/compiling/translating (which would be the right word here?) to ES5. Browserify with es6ify has made the workflow quite easy (same as Browserify + coffeescript).

Reply on news.ycombinator.com