eugeneware / debowerify

A browserify transform to enable the easy use of bower components in browserify client javascript projects. This can be used in conjunction with deamdify to require AMD components from bower as well.
493 stars 51 forks source link

Enable Bower API options #56

Closed JorritPosthuma closed 9 years ago

JorritPosthuma commented 9 years ago

I needed te be able to set the cwd option of the Bower API. This enables that.

tellnes commented 9 years ago

Is there any reason not to pass the options argument directly to bower?

It can be done in combination with something like this:

if (typeof options.offline === 'undefined') options.offline = true

Btw, you do not need to create a new pull request when doing modifications. You can just update the branch you are creating the pull request from.

eugeneware commented 9 years ago

Yeah, I think passing the options directly might be the way to go.

sukima commented 9 years ago

I tend to prefer testing undefined / null this way:

if (options.offline == null) options.offline = true;

That catches both null and undefined. You can also simplyfy conditions that depened on false value as

if (options.offline !== false) doSomething

That forces the default regardless of type or value unless false was explicitly assigned to turn it off.

eugeneware commented 9 years ago

the typeof xx === 'undefined' is the recommended safe way to test for undefinedness, and is also recommended by the the most excellent tome effective javascript

It's verbose, but nice and safe.

You also want to make sure that the options object is an object as well, so you don't throw an error if the options object is undefined?

JorritPosthuma commented 9 years ago

Better?

sukima commented 9 years ago

Testing typeof is only needed if your testing the existence of a possibly undeclared variable. Since all objects will return undefined for missing properties it's essentially superfluous.

eugeneware commented 9 years ago

LGTM :+1:

eugeneware commented 9 years ago

Thanks @JorritPosthuma for the PR! I've merged and published this as 1.2.0. I've also added you to the contributors list and giving you collaborate access to the repo as per our Open Open Source Contribution Policy.

Cheers!