Sails version: 0.12.1
Node version: 5.10.1
NPM version: 3.8.3
Operating system: OSX / Debian
At the moment sails uses hard paths and always assumes node_modules is located at appPath/node_modules - this is hard coded in a lot of places (path.resolve and require), i.e. hook loading from package.json files.
If you wanted to completely write your sails project in ES2015 onwards or something like coffee script 馃檴 (by completely I mean everything including config, services etc) your options are limited, for example writing a complete ES2015 project in sails would require using the babel require hook - which for production is way too heavy.
node_modules/
...
src/
api/
...
config/
...
app.js
build/
api/
...
config/
...
app.js
.sailsrc
package.json
And then to lift my app I run node build/app.js.
I have a npm script which just watches for changes and re-compiles the affected files.
"build-watch": "babel src --watch --out-dir build --copy-files --source-maps inline"
I am aware you can set appPath in the sails config - which works fine. But there is no way of overriding the node_modules path as it's all hard coded. Managed to make a work around...
// src/app.js
// this assumes your build directory is /build
import Module from 'module';
import Path from 'path';
const originalRequire = Module.prototype.require;
const originalResolve = Path.resolve;
// override resolve to intercept invalid sails node_module paths
Path.resolve = function () {
const args = [].slice.call(arguments);
if (args.length >= 2 && typeof args[0] === 'string' && typeof args[1] === 'string') {
if (args[0].endsWith('/build') && args[1] === 'node_modules') {
args[1] = './../node_modules'; // force resolve to go up one dir
}
}
return originalResolve.apply(this, args);
};
// override require to intercept invalid sails node_module require paths
Module.prototype.require = function () {
const args = [].slice.call(arguments);
if (args.length && typeof args[0] === 'string') {
if (args[0].includes('/build/node_modules/')) {
args[0] = args[0].replace('/build/node_modules/', '/node_modules/');
}
}
return originalRequire.apply(this, args);
};
(function () {
require('sails').lift(Object.assign(require('rc')('sails'), {
appPath: __dirname // set the appPath to this dir - so when compiled it's projectDir/build
}));
})();
This works well, but I'm just wondering is there not a better way to do this? Could sails not have an option to set the node_modules path the same as all the other paths at sails.config.paths (but this is everywhere though and in other modules, so probably a lot of changes to get this working).
Thoughts?
Danke!
The node_modules/ convention is a pretty pervasive one, and playing around with that would affect an important part of how Node.js works. That said, I'm always down to experiment w/ making this kind of thing customizable as long as all of the out-of-the-box conventions still work (and assuming it's done in a way that's well tested and easy to maintain).
However:
(but this is everywhere though and in other modules, so probably a lot of changes to get this working).
You pretty much hit the nail on the head there-- I'd be pretty reticent to commit to a configurable node_modules path as part of core simply because so many of our dependencies rely on dependencies living there.
Despite that, I think there might be another way to do this without making the node_modules/ path configurable-- not sure though, because I'm not 100% up to speed on the reasoning behind needing to change the path. @Salakar I'd be curious to hear your thoughts about https://github.com/balderdashy/sails/pull/3142
And btw, thanks for sharing your workaround! I'm sure it'll help some other folks out over time.
Thanks for posting, @Salakar. I'm a repo bot-- nice to meet you!
The issue queue in this repo is for verified bugs with documented features. Unfortunately, we can't leave some other types of issues marked as "open". This includes bug reports about undocumented features or unofficial plugins, as well as feature requests, questions, and commentary about the core framework. Please review our contribution guide for details.
If you're here looking for help and can't find it, visit StackOverflow, our Google Group or our chat room. But please do not let this disrupt future conversation in this issue! I am only marking it as "closed" to keep organized.
Thanks so much for your help!
If I am mistaken, and this issue is about a _critical bug with a documented feature_, please accept my sincerest apologies-- I am but a humble bot working to make GitHub a better place. If you open a new issue, a member of the core team will take a look ASAP.
Thanks for the response @mikermcneil
I went into my reasoning a bit more in my response on #3695 - much better to be able to use the same JS feature set when doing a isomorphic setup.
But I guess this was just more of a query in the end. Come to think of it, in my use case this would be short term usage potentially anyway - for a start: Node v6 RC is due for release later this month containing V8 version 5 - which I believe has 97% ES2015 support, but don't quote me on that 馃対
As for #3142 - I'll respond on there with thoughts
@Salakar many thanks from 2020 for the workaround.
For reference, in sails 1.3.1 I solved this with a modification of @Salakar's code:
const Path = require('path');
// override resolve to intercept invalid sails node_module paths
Path.resolve = (() => {
const originalResolve = Path.resolve;
const absoluteAppPath = Path.resolve(appPath);
return function(...args) {
if(args[0] === appPath || args[0] === absoluteAppPath) {
if(args[1] === 'node_modules' || args[1] === 'package.json') {
args = [ args.shift(), '..', args.shift(), ...args ];
}
}
return originalResolve.apply(this, args);
};
})();
The package.json override may or may not be required, depending on use case.
Most helpful comment
@Salakar many thanks from 2020 for the workaround.
For reference, in sails
1.3.1I solved this with a modification of @Salakar's code:The
package.jsonoverride may or may not be required, depending on use case.