search

broccolijs / broccoli-funnel

MIT License
66 stars 37 forks source link

Why no optional `new`? #18

Closed joliss closed 9 years ago

joliss commented 9 years ago

I was just trying to update the example in the Broccoli README to use funnel instead of static-compiler, and I noticed that broccoli-funnel doesn't support new-less instantiation. Before I put new Funnel on the Broccoli README (which is a bit different from how most plugins are instantiated), I figured I'd just ask, is there a reason why you don't like the newless style?

I'm attaching a PR in case you want to allow it.

I goes well with a verbey invocation style as well, using funnel as in "to funnel":

var funnel = require('funnel');
var newTree = funnel('foo', ...);
rwjblue commented 9 years ago

It was definitely intentional, though I suppose it might be possible to convince me otherwise.

My main reasoning is that I believe calling new Funnel(....) makes it much clearer that I am dealing with an instance of the Funnel class, whereas calling funnel(...) is a complete black box (even though it would call new internally). I also dislike having to add the standard boilerplate to all plugins (though I haven't been super consistent in this stance so some of my plugins require new and others do not).

rwjblue commented 9 years ago

A little more detail in https://github.com/rwjblue/broccoli-funnel/pull/5.

joliss commented 9 years ago

So my reasoning was, new is basically an implementation detail. The function-call Brocfile style was meant to evoke a functional-programming style with immutable objects: Call a function on an (immutable) input tree, get a new (immutable) output tree.

Agree on the "boilerplate in all the plugins" problem though.

To my mind, it's kindof a tossup - but either way it'd be nice if we can converge on some agreement so our plugins are more consistent.

rwjblue commented 9 years ago

it'd be nice if we can converge on some agreement so our plugins are more consistent

This is the winning argument. Lets document that this is "heavily recommended" in the Broccoli docs?

joliss commented 9 years ago

Done (https://github.com/broccolijs/broccoli/commit/0463dc9eaf34363915586b0adc0e65500b981076). Hm, the plugin spec is clearly in need of some love, and also needs to be updated for the new API. Perhaps it's just lacking examples - we could add a brief "how to write a plugin" tutorial thing. Anyways...

Do you want me to update the broccoli-funnel README as well to use the newless style, or do you want to keep it?

I'm happy to move funnel to broccolijs if you want, and add you as maintainer either way

Moving to broccolijs sounds great. Funnel seems pretty important.

joliss commented 9 years ago

Do you want me to update the broccoli-funnel README as well to use the newless style, or do you want to keep it?

I'm sending a PR at #19 in case you want it.