substack/node-browserify — pull request #717 by codelahoma (2014-04-04 21:39:57+00:00)
Title: Current implementation breaks if `opt.builtins` is set to anything other than `false` (not falsey)
or an object similar to lib/builtins.js, and only actually *does
anything* in the `false` case.

The current documentaion describes `opt.builtins` as "set[ting] the list of builtins to use",
which sounds to me like it would be a whitelist array of builtin names
as strings, but the CLI defaults it
to `true`, but always sets it to `false` or `undefined` when calling the
API.

Given all this, and an admittedly incomplete knowledge of the use cases
on my part, I propose the following spec for `opt.builtins`, which this
pull request implements.

`opt.builtins` | Behavior
----------|---------
`false` | exclude all builtins (current behavior)
`true` | no effect (currently breaks)
`["builtinname",...]`| exclude all builtins not named in the array (empty array behavior matches `false`)

The `true` option, while not strictly necessary, provides symmetry as
well as allowing the use of a boolean defined elsewhere to control
simple inclusion/exclusion of builtins (eg gutil.env.node)

Any other value (object, number,..) will have no effect, effectively matching the `true`
behavior. For some reason, deciding what they would do was just one
assumption too many for me. :-)

I've modified the `no_builtins` test to also check the empty array case,
but I need some direction on how to write the tests for the whitelist
cases.

Any feedback would be appreciated, especially on how to
build those tests.