Closed srquinn21 closed 10 years ago
Awesome, merged. I like it - this means that much of the existing documentation is usable but people have a reasonable default to work off of. I spotted a minor issue with the internals which would be nice to fix: namely, if files are moved (after starting the server) then a couple of errors are raised. In some sense that's expected but I'll probably fix it later.
One thing I'd love to have as well is some default handling of the node_modules though that probably requires that we do static analysis of require statements. Right now one can do
app.use('/js/app.js', glue.middleware({
include: [ './lib', './node_modules/backbone/' ]
}));
which is pretty decent.
Sweet – glad to contribute! I can throw a test suite together and another PR in the next week or two.
I didn't account for the edge case of moving files after the server starts, though its probably a good one to consider if this were to back something like a package manager. Would it be too costly to setup file watchers for each include?
Also, while putting this together I played with the idea of including the ./node_modules
directory by default but found the exclude rules became daunting when filtering server packages from browser packages. I couldn't come up with a reasonable way to solve this without maintaining a list – less than ideal.
It is, however, reasonable to assume that server packages are only for development and therefore belong in the devDependencies
list in package.json
. You could define an option to filter the packages based on this field. If you're interested, I'll see about putting it in the aforementioned PR.
Cool, I just updated the readme and published this as v2.2.0
The way I've been wanting to do the whole inclusion from node_modules is to incorporate information from https://github.com/substack/node-detective which does the necessary static analysis to identify how things depend on each other. It's a bit slow if run naively (e.g. every time on every file) but the results could be aggressively cached and provide for even more accurate filtering and the ability react to code changes by including more modules from node_modules
.
My initial thought after finishing up is that this functionality belongs in a plugin/component and not core because it consumes the API rather than adds to it. However, the marketeer in me also knows that
gluejs
won't survive the unsteady waters of node module development unless the n00bs can plug-n-play. With that said, this feature is pretty straight forward and a good ground floor to begin supportingexpress
out of the box.I originally had a highly opinionated set of defaults that I eventually neglected for simplicity in setup. The interface I propose here only requires one parameter and all other options are left as opt-in – define them if you need them, ignore them and they'll be set to
glue
's defaults. This gets you up on your feet quick.Middleware can be defined with
app.use()
:Or at the route level:
The
basepath
defaults to theinclude
path andmain
defaults to anindex.js
script in theinclude
directory, so each of the above will default to:And can be requested in HTML thusly:
The necessity for defining an include could be stripped out if we assume a convention of using
lib/
directory in thecwd
for main include:But this seemed a little opinionated and I wanted you to weigh in first so I added it as a separate commit. Let me know how this looks and if there is anything else you would like to consider.