search

HenrikJoreteg / moonboots

A set of conventions and tools for bundle and serving clientside apps with node.js
158 stars 20 forks source link

Also pass browserify config on browserify init #44

Closed alappe closed 10 years ago

alappe commented 10 years ago

Some options mentioned in browserify-documentation can only be set on the init of browserify, not passed on the bundle method. Now both is done…

alappe commented 10 years ago

At least I didn't find any other way to get something like transforms: ['coffeeify'] and extensions: ['.coffee'] through to browserify…

Sadly, I also didn't see any way to write a test for this.

wraithgar commented 10 years ago

Is doing both the ideal behavior? Currently transforms are being manually ran through bundle. If browserify can do them automatically during bundling do we perhaps want to see if that can be done instead (i.e. just passing the config to browserify now)?

lukekarrys commented 10 years ago

Also, those docs are to the latest version of browserify which changed the behavior. I think we should hold off on this until we update to browserify v5 in #39.

alappe commented 10 years ago

The docs I linked to are from v4.2.3 which is the current version moonboots is using. While I would prefer an update to v5, I thought this might take you longer and this patch is a quickfix for me to use ampersandjs with moonboots and CoffeeScript.

I am actually not sure if doing both is ideal. In v5, browserify doesn't take any config on bundle() anymore, but reading the v4.2.3 docs, I understand that options like extensions (which I need for CoffeeScript) can only be passed on init.

lukekarrys commented 10 years ago

Sorry about that @alappe! I totally missed the version on your docs link. My bad! :grimacing:

I was looking at browserify docs for v5 most of the day, and actually have it working in a branch. So hopefully we can get that merged soon :smile:

The only reason I'm hesitant to merge this in the meantime, is if there could be any negative side effects to applying the config in both places.

alappe commented 10 years ago

@lukekarrys No harm done ;-)

As I said, I'd prefer v5 too, so if you say it's close, let's go for that…

lukekarrys commented 10 years ago

So there were some setbacks with the browserify v5 stuff in #46 and #47. Since all the tests pass on this PR, I'm tempted to merge and publish.

I never realized before that there were browserify options you couldn't pass to moonboots, so I do think this should be fixed since things like extensions are currently impossible to implement.

@wraithgar What are your thoughts?

wraithgar commented 10 years ago

Is there a way to make sure transforms are not being ran twice with this approach?

lukekarrys commented 10 years ago

There's a test for transforms and I updated it to check for number of times ran, and it only runs once.

lukekarrys commented 10 years ago

Published in 2.1.0.