ForbesLindesay / browserify-middleware

express middleware for browserify, done right
http://browserify.org
MIT License
381 stars 66 forks source link

Accept a browserify bundle. #23

Open mcollina opened 10 years ago

mcollina commented 10 years ago

I think a more convenient method for supporting the full browserify API is to accept a builder function:

var bmid = require('browserify-middleware');
var browserify = require('browserify');
var express = require('express');
var app = express();

app.use('/js/bundle.js', bmid(function() {
  return browserify('beep');
}));

app.listen(3000);
ForbesLindesay commented 10 years ago

It's an option. It's not actually quite as powerful as you'd think though. Browserify lets you put some options in the call to .bundle so we's probably end up needing to jsut accept a stream. We'd then loose 90% of the functionality this module adds (e.g. debug mode in development/minify in production)

mcollina commented 10 years ago

The reason I would like to have this is for not having two different initialization logic regarding paths. I wanted to expose a module and the only way I've found was: https://github.com/mcollina/mosca/blob/master/lib/server.js#L349-L351

Passing just the module name won't work if the app is started from outside its directory.

ForbesLindesay commented 10 years ago

Have you tried passing the fully resolved names to browserify-middleware? This really should be achievable without browserify-middleware accepting a bundle.

mcollina commented 10 years ago

Exactly, but that's horrible, and very undocumented in browserify-middleware.

https://github.com/mcollina/mosca/blob/master/lib/server.js#L392-L396

ForbesLindesay commented 10 years ago

You could just do:

Server.prototype.serveBundle = function(app) {
  app.get('/mqtt.js', browserify('mows', { standalone: 'mqtt' }));
};

Unless there's also a library at __dirname + '/node_modules/mows/index.js' it should resolve to the same thing.

What would hat code look like if I supported your feature?

mcollina commented 10 years ago

That was not resolving 2 months ago unless the app was run from directly inside the mosca package directory, as for some reason it could not look up mows correctly. It was looking at the directory where the executable was launched, e.g. pwd.

ForbesLindesay commented 10 years ago

It should work now, I don't know why it didn't when you first tried it. If it doesn't you can always do:

Server.prototype.serveBundle = function(app) {
  app.get('/mqtt.js', browserify(require.resolve('mows'), { standalone: 'mqtt' }));
};
mcollina commented 10 years ago

It keeps failing badly:


fs.js:684
  return binding.stat(pathModule._makeLong(path));
                 ^
Error: ENOENT, no such file or directory '/Users/matteo/Repositories/mosca/lib/mows'
    at fs.statSync (fs.js:684:18)
    at browserify (/Users/matteo/Repositories/mosca/node_modules/browserify-middleware/index.js:29:7)
    at EventEmitter.Server.serveBundle (/Users/matteo/Repositories/mosca/lib/server.js:393:23)
    at Server.async.series.server (/Users/matteo/Repositories/mosca/lib/server.js:135:16)
    at /Users/matteo/Repositories/mosca/node_modules/async/lib/async.js:548:21
    at /Users/matteo/Repositories/mosca/node_modules/async/lib/async.js:224:13
    at iterate (/Users/matteo/Repositories/mosca/node_modules/async/lib/async.js:131:13)
    at /Users/matteo/Repositories/mosca/node_modules/async/lib/async.js:142:25
    at /Users/matteo/Repositories/mosca/node_modules/async/lib/async.js:226:17
    at /Users/matteo/Repositories/mosca/node_modules/async/lib/async.js:553:34

However with the latest version it works smoothly with require.resolve. Thanks!

ForbesLindesay commented 10 years ago

What's the exact code at /Users/matteo/Repositories/mosca/lib/server.js:393:23 Note that it must be 'mows' that you pass to browserify and not './mows' It uses the module resolution algorithm from node.

mcollina commented 10 years ago

This is the branch that it is not working: https://github.com/mcollina/mosca/tree/browserify-middleware-issue.

If you run the 'start.sh' script inside the demo/ folder, the resolution algorithm fails: https://github.com/mcollina/mosca/blob/browserify-middleware-issue/demo/start.sh

ForbesLindesay commented 10 years ago

Ah, I thought we were using the node.js resolution algorithm, turns out we do just join the paths, so you were right all along, my apology.

mcollina commented 10 years ago

:dancers: :D. No issue, it's not really urgent, the 'require.resolve' trick works smoothly.

riccardo-forina commented 9 years ago

Upping this because this - or something like this - is needed to support es6ify (http://thlorenz.github.io/es6ify/).

es6ify needs browserify to be configured like this to work:

browserify()
  .add(es6ify.runtime)
  .transform(es6ify)
  .require(require.resolve('./src/main.js'), { entry: true })
  .bundle({ debug: true })
  .pipe(fs.createWriteStream(bundlePath));

I played around with current api, but looks like there is no way to get this result.

Any idea?