Expose all or a portion of the node_modules projects in the public folder under the node_modules folder #330
Comments
* Link in used npm package into the public directory. This is not the only way to do this but is just a sample. Applies to OpenUserJS#330
|
@Martii commented on 27 aug. 2014 00:49 CEST:
Can you elaborate what you mean with this issue a little more, I don't understand (probably the language difference). |
|
I'll look this over later today. I've done something similar at gamerpolls (for moment.js) which is basically just using the express.static middleware: // GET /javascripts/jquery.js
// GET /style.css
// GET /favicon.ico
app.use(express.static(__dirname + '/public'));or more specifically, to namespace each module: // GET /static/javascripts/jquery.js
// GET /static/style.css
// GET /static/favicon.ico
app.use('/static', express.static(__dirname + '/public'));So you could do something like this to expose a specific module really easy: app.use('/node_modules/select2', express.static(__dirname + '/node_modules/select2')); |
Look at this line of code and see how the select2 package is available on npmjs.org, and potentially referenced, is resolved here and the whole project is linked in here. Currently we "cherry pick" a few files and place them in our public folder for use in our client side JavaScript that we generate. This has a disadvantage of needing to do this "cherry picking" every dependency update time. select2 isn't the only package. Pretty much all the static copies of these projects are used with client side generation of some stuff we do. select2, for example, is currently used for selecting the "Groups" that a script belongs to with a dropdown list that parses our current group lists. The advantage is that since it's already on npmjs.org we just link it in instead of having it as part of a build/exec/deploy system and let npm handle the updating of the actual package. Some packages have a Alternatively we could build a dictionary list of what needs to be linked in a json file for mapping from the npm package to our public directory. So when updating dependencies we would need to figure out exactly what is needed to make sure select2 is fully functional. There are multiple ways to do this for the packages that are on npmjs.org already but we manually copy them currently. Referencing them instead would be an improvement I think since they would already be on there in the first place... e.g. select2 right now is available on nodejitsu, we just don't include it in the Does this help more? |
app.use('/node_modules/select2', express.static(__dirname + '/node_modules/select2'));Thanks for posting one of the options available for linking the whole dependency project in at once using the available express routines. :) There's usually quite a few ways to do this. |
|
@Martii commented on 27 aug. 2014 20:16 CEST:
Thnx. Seems like something that makes us life a little easier. @cletusc commented on 27 aug. 2014 20:16 CEST:
I've done something similar: https://github.com/jerone/CrystalMinesHTML5/blob/master/server.js#L24-26 |
|
There's also (Untested) var staticfile = function(filepath) {
return function(req, res, next) {
res.sendfile(filepath);
});
};
app_route(staticfile('/js/select2.js').get(staticfile('./node_modules/select2/select2.js'));But we'd probably need to use the |
|
I really like the If everyone takes a look at their output with: console.log(module);Mine looks a little like this currently: { id: '.',
exports: {},
parent: null,
filename: '/home/user/repo/git/oujs/martii/OpenUserJS.org/app.js',
loaded: false,
children: [],
paths:
[ '/home/user/repo/git/oujs/martii/OpenUserJS.org/node_modules',
'/home/user/repo/git/oujs/martii/node_modules',
'/home/user/repo/git/oujs/node_modules',
'/home/user/repo/git/node_modules',
'/home/user/repo/node_modules',
'/home/user/node_modules',
'/home/node_modules',
'/node_modules' ] }you might see multiple Secondly We can probably "mash up" all these methodologies to get the right values so on a deploy to any server, locally or remotely, it doesn't bomb. |
When we specify a package in the The Node doesn't do any special linking of files or folders.
Anything can be made global for all files, though we should avoid it if at all possible (for the same reasons as browser globals), as it is a nightmare for maintenance. // globals.js
global.foo = function () { return 'bar'; };
// someotherfile.js
// Must be done only once somewhere in the chain before anything is called.
require('./globals');
foo();
// 'bar'IMO, we should explicitly expose all modules in a single location for maintainability:
...as well as be as specific as possible, e.g. down to the folder containing the file ( |
If you mean in the |
Although I agree that this could potentially allow others to use our files on their own servers, I think that is a separate issue as all of our public files can be used remotely. Protecting public files from third party use shouldn't be the concern for this issue. I don't see why a module can't be served everywhere, then only actually link (via |
You are correct in that all public files can be used remotely even before this issue and even before entry into this project... however the
As Zrens already done with You're issue at #321 definitely opened up Pandora's box. ;) :) |
|
So thinking "always there" for the moment... @Zren The following sample tested and corrected code above of yours works as: var path = require('path');
var staticfile = function (aFilepath) {
return function (aReq, aRes, aNext) {
aRes.sendfile(aFilepath);
};
};
[
'select2.js',
'select2.css',
'select2.png',
'select2x2.png',
'select2-spinner.gif'
].forEach(function (aElement, aIndex, aArray) {
app_route(path.join('/node_modules/select2', aElement)).get(staticfile(path.join('./node_modules/select2', aElement)));
});but you say that for the Ace editor we should use the method presented by @cletusc since it's rather large. @cletusc var path = require('path');
[
'select2.js',
'select2.css',
'select2.png',
'select2x2.png',
'select2-spinner.gif'
].forEach(function (aElement, aIndex, aArray) {
aApp.use(path.join('/node_modules/select2', aElement), express.static(path.join(__dirname, '/node_modules/select2', aElement)));
});but also works with just the folder of select2 itself. e.g. if we wanted to do any whole package. I see the Zren method has the advantage of chainability and I see the Cletusc method has the advantage of no intermediate function (yet) but not chainable. Did I get this visualization/wording for a summary correct? So what I think it "boils down to" for "always there" is do we really need those chainable or not? Preference anyone? |
|
Didn't realized |
|
Remember I'd like to add caching and minifying when necessary (EDIT: and the referer header check too at least for the FQDN). I know how to do this in Apache/PHP and I'm in the process of translating my knowledge base to Node/express. I'd rather not cut us short accidentally before we get started with that. |
|
@Zren Minificiation I found a package, that I still have to twiddle more with, that alleges binding the output of .js and .css files. I believe using |
// Accepts exact same params as `express.static`.
function duckPunchedStatic() {
// Punch that duck in the face.
var punched = express.static.apply(this, arguments);
// Return same signature as the duck you punched.
return function (req, res, next) {
// Do stuff here with req and res.
// ...
// Let the duck do its thing now, no more punching.
return punched(req, res, next);
};
};
app.use('/mountpath', duckPunchedStatic('/node_modules/ducks/'));which serves these headers on a test file of mine: and even setting your own headers (and a max age through // Accepts exact same params as `express.static`.
function duckPunchedStatic() {
// Punch that duck in the face.
var punched = express.static.apply(this, arguments);
// Return same signature as the duck you punched.
return function (req, res, next) {
// Do stuff here with req and res.
// ...
// Status of the duck head(er)?
res.set({
'X-Duck-Status': 'Officially PUNCHED'
});
// Let the duck do its thing now, no more punching.
return punched(req, res, next);
};
};
app.use('/mountpath', duckPunchedStatic('/node_modules/ducks/', {
// Only allow it to be cached for an hour.
maxAge: 1000 * 60 * 60
}));which serves these headers (success!):
|
Besides being the most unusual nomenclature I have seen in a while (ducks and punches) that methodology appears to work so far: EDIT: function serve() {
var fn = express.static.apply(this, arguments);
var maxAge = 1000 * 60 * 60; // Default
var minify = false; // Default
if (arguments && arguments[1]) {
maxAge = arguments[1].maxAge ? arguments[1].maxAge : maxAge;
minify = arguments[1].minify ? arguments[1].minify : minify;
}
return function (aReq, aRes, aNext) {
if (process.env.NODE_ENV === 'production') {
if (!/^https:\/\/openuserjs\.org\//.test(aReq.headers.referer)) {
return aNext();
}
}
aRes.set({
'Cache-Control': 'public, max-age=' + maxAge
});
return fn(aReq, aRes, aNext);
};
};[
'select2.js',
'select2.css',
'select2.png',
'select2x2.png',
'select2-spinner.gif'
].forEach(function (aElement, aIndex, aArray) {
aApp.use(
path.join('/redist/npm/select2', aElement),
serve(path.join(__dirname, '/node_modules/select2', aElement), { maxAge: 5000 })
);
});Remembering that I'm more PHP aware than express a few questions come to mind:
I still have to test this with static served Ace but it is what I have at the moment for select2.... EDIT: please keep in mind this is just a test. |
|
Some miscellaneous notes "out loud": Syntax wise I'm having to return Testing these packages for minification at the moment: Besides
Piler is nice but it requires us to use drone memory just to load any package into server side memory... that is trying to be avoided. |
|
I'm working on a separate response shortly. In short, minification shouldn't be part of this middleware. Serving files and minifying should be two separate middleware operations. |
|
I hope most of this makes sense--my headset broke so I became a bit preoccupied while writing this. :( I've marked up your code with some comments ( function serve() {
var fn = express.static.apply(this, arguments);
var maxAge = 1000 * 60 * 60; // Default
var minify = false; // Default
// CletusC [1]: This needs to happen before `express.static.apply`.
if (arguments && arguments[1]) {
maxAge = arguments[1].maxAge ? arguments[1].maxAge : maxAge;
minify = arguments[1].minify ? arguments[1].minify : minify;
}
return function (aReq, aRes, aNext) {
// CletusC [2]: Be careful with this, it will run on all paths unless filtered further with a mount path.
if (process.env.NODE_ENV === 'production') {
if (!/^https:\/\/openuserjs\.org\//.test(aReq.headers.referer)) {
return aNext();
}
}
// CletusC [3]: Not needed, `express.static` handles this with the `maxAge` arg.
aRes.set({
'Cache-Control': 'public, max-age=' + maxAge
});
return fn(aReq, aRes, aNext);
};
};
[
'select2.js',
'select2.css',
'select2.png',
'select2x2.png',
'select2-spinner.gif'
// CletusC [4]: aIndex and aArray are not needed unless we actually use them.
].forEach(function (aElement, aIndex, aArray) {
aApp.use(
// CletusC [5]: IMO should be marked as /node_modules/select2/--keep it the same as whatever the actual folder is.
path.join('/redist/npm/select2', aElement),
serve(path.join(__dirname, '/node_modules/select2', aElement), { maxAge: 5000 })
);
});
Now, I would recommend we separate as much logic as possible from what should just be serving files. Here is a tested (although not super thorough) solution that would serve up a particular module's folder. function serveModule(aApp, aExpress, aModuleName, aOptions) {
var path = require('path');
var modulePath = null;
var separator = path.sep + path.sep;
var rBaseModulePath = new RegExp('(node_modules' + separator + '[^' + separator + ']+?' + separator + ').*$');
// Check required args.
if (!(aApp && aApp.use) || !(aExpress && aExpress.static) || !aModuleName) {
throw new Error('invalid arguments');
}
// Make sure options is an object.
if (typeof aOptions !== 'object') {
aOptions = {};
}
// Make sure files has at least one element.
if (!Array.isArray(aOptions.files) || aOptions.files.length < 1) {
aOptions.files = [''];
}
// Get module path.
modulePath = require.resolve(aModuleName);
// Get base folder path.
modulePath = modulePath.replace(rBaseModulePath, '$1');
// Loop through each file.
aOptions.files.forEach(function (aFile) {
var fullPath = null;
var mountPath = null;
if (typeof aFile !== 'string') {
return;
}
// Get paths.
fullPath = path.join(modulePath, aFile);
mountPath = '/node_modules/' + aModuleName + '/' + aFile;
// Serve the files.
aApp.use(mountPath, aExpress.static(fullPath, aOptions.staticOptions || {}));
});
}
// Usage.
serveModule(app, express, 'select2', {
files: [
'select2.js', // node_modules/select2/select2.js is accessible.
'select2.css',
'select2.png',
'select2x2.png',
'select2-spinner.gif'
],
staticOptions: {
maxAge: 1000 * 60 * 60 // 1 hour
}
});
serveModule(app, express, 'select2'); // node_modules/select2/ is accessible.As for minification of files and checking referers, we should handle it separately like this (not tested, just for concept): // Handle minification during production.
var minify = require('express-minify');
if (process.env.NODE_ENV === 'production') {
app.use(minify());
}
// Check referer.
app.use(function (aReq, aRes, aNext) {
// ...
});
// Handle the served modules.
serveModule(...);
// Handle all the express.static stuff.
app.use(express.static(...));
// Handle any other app.* stuff.
// ...Logic is thus separated and more maintainable. Need something to do with minifying? Go to the minifying middleware. Need to serve something? Use one of the serving middlewares. Need to allow a referer, or whitelist a particular file? Go to the referer middleware. |
|
This will take me some time to digest in full but I noticed that you are doing way more error checking than what I've posted... that is a later thing.
express-minify isn't going to cut it... not enough options... we need to be able to control it down to the exact minification for applicable files. Using a car reference here... seems more like an "idiot light" instead of a real gauge.
So you are basically saying that we should have another abstraction layer on top of the already abstracted middlewares we require then... right? The problem there is minifying and caching... both of those have to be done before the file is served client side... so somewhere in Anyhow thanks for the reply and I'll look it over in massive detail in the next day or so. |
|
btw the: [
'select2.js',
'select2.css',
'select2.png',
'select2x2.png',
'select2-spinner.gif'
]bit is most likely going to need to include single file options for cache time value, whether or not to minify in our current |
Applies to OpenUserJS#330 and required by OpenUserJS#255
|
I don't want to open another issue, because it is maybe me, but I'm getting 404 with this I understand what is happing and how it should work, but every file going through the I have confirmed that the files exists. Anyone ( @Martii ) have any idea... did I miss something? |
It's most likely the express package update in #373... I get it every great once in a while and a page refresh does the trick. The solution is to walk the version back one version at a time and see where it doesn't show up at all on production (this is a pro test only since dev never has this issue here much like the aws-sdk package was in #395) ... ...however it would be great to understand why we aren't using express 4.x... sizzle already stated that he wants it at express 3.x... but I don't know why. |
|
Updating express 3.18.2 did not solve my issue. Anyone else on Windows having this issue? |
|
You redid a |
|
@Martii commented on 6 nov. 2014 22:31 CET:
Yes. I confirmed having another version the before. |
|
I also have tried the following without success, to be sure it's not a path on Windows issue: |
|
Hmmm Well again in Linux dev it works peachy here using: $ node -v
v0.10.32
$ npm --version
2.1.5
$ rm -Rf node_modules
$ npm install
...
26 passing (29ms)
...
$ node app.jsand visiting http://localhost:8080/redist/npm/font-awesome/css/font-awesome.min.css and even on pro with http://openuserjs.org/redist/npm/font-awesome/css/font-awesome.min.css. $ node app.js
connect deprecated methodOverride: use method-override npm module instead app.js:53:17
GitHub client authenticated
GET /redist/npm/font-awesome/css/font-awesome.min.css 200 8561.558 ms - -Have you cleared your cache recently? e.g. maybe you are getting hung up in the browser? I do see that |
I can add the |
|
The following code seems to work: Trying to debug the |
|
Oh finally found the solution; change https://github.com/OpenUserJs/OpenUserJS.org/blob/master/routesStatic.js#L25 to the following: Can someone confirm that this works for them too... I haven't found the reason behind it yet... |
|
Reopening since there appears to be an issue on Windows dev. |
|
The first argument of |
* Fix Windows dev from not serving module/module components * Use some standard node nomenclature for identifier naming * Use **native** node [url](http://nodejs.org/api/url.html) * Reorder `require`s to have natives first then third party packages after * Stray comma removed Re - Closes OpenUserJS#330
|
Please remember https://github.com/OpenUserJs/OpenUserJS.org/blob/master/CONTRIBUTING.md#pull-request-process Item 1 subitem 1 ... I'm still actively watching this for discussion (and now I get to restart watching for a boog that intermittently shows up but I suspect it's express) hence the |
* Link in hljs css for GitHub styling ... missed from OpenUserJS#330 ... glad to whittle the CSS down a bit more :) * Remove our dated static copy of GH highlight rules from OpenUserJS#22 * Delete ops retested See their changelogs for exacts... mostly bug fixes, some doc fixes, some refactoring
For the static projects that we use and copy in manually that have npm packages... expose at the very least those packages in entirety and at most all of them in entirety. The latter of course is easier.
I'd like a list of PROs vs CONS please. I've given this quite a bit of thought so be ready for responses. As usual since I'm only human I may have missed something... which is why this is team biz.
The primary reasoning behind this is to close #321 by using exact versions and is the only reasonable reason to use exact versions that I have found. Having a third party monitor our need to update is a perk. This would also enable npm to handle those updates and we could just debug/maintain JUST our code and those projects that aren't npm ready. e.g. lighten the load. This is the equivalent to using production as the CDN like we currently are with static copies but no need to copy related package bits. We can revisit alternate CDNs if it gets too slow. So far #324 is showing no change in existing speed stability... so this issue is the next progression chain.
Think about some and get back to this issue please. e.g. think deep. Assigning to self for the time being and should be a consideration for #262.
The text was updated successfully, but these errors were encountered: