
Node.js Best Practices - finspin
https://www.joyent.com/developers/node/design
======
Touche
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.

~~~
chrisfosterelli
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.

~~~
Monkeyget
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](https://www.joyent.com/developers/node/design/errors)

~~~
jhchen
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).

~~~
nawitus
>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.

------
general_failure
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.

~~~
Touche
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.

~~~
jwmerrill
It's to avoid using "new".

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

~~~
aikah
> 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.

~~~
greg5green
The answer is yes if you do:

    
    
        Foo.call({});

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

~~~
cygx
Yup. The proper way to emulate _new_ is

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

~~~
LazerBear
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.

~~~
jdlshore
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).

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

------
Kiro
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.

~~~
jb55
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](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.

~~~
Touche
"small"

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

------
whatthemick
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.

~~~
wooptoo
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.

------
pavlov
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.

------
pedalpete
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?)

~~~
akbar501
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(); }

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

~~~
kansface
Closures are fine, but avoid seagulls:

function() {

    
    
      function() {
    
         function() {
    
            function() {
    
            }
    
          }
    
       } 
    
    }

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

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

------
EGreg
Very useful!

------
_random_
0\. Avoid using it, unless truly necessary.

------
passfree
I have one word: CoffeeScript.

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

~~~
iLoch
Typewriter..?

~~~
insky
Hairsplitter.

