sails.js fails if node_modules is not in same directory as the application

[kevinburke]

The node.js application supports loading node_modules from a different folder via the NODE_PATH environment variable.

That way, I can put node_modules for my application in /usr/lib/node_modules (or wherever) and have my app in /opt/myapp and then run it with NODE_PATH=/usr/lib/node_modules node whateverscript.

Why would anyone want to do this? Well... we run Macs, but have an Ubuntu VM with Sails inside, so we can have a reproducible development environment, isolation, mirror what we run in production, etc. etc.

We have a shared folder in the VM which shares our application folder between the Mac and the VM. Running Sails with a node_modules directory in this shared folder was very very slow; on a brand new MacBook Pro, it was taking about 1 minute and 40 seconds to evaluate require('sails').load({verbose: true}). We don't have a crazy Sails application; maybe ~100 routes and ~50 models. (Running strace on our test runner showed that there were ~33,000 calls to stat; node is doing a lot of something to run the tests.) Anyway as you might imagine, hopping in and out of the VM this many times is slow and we were able to speed up require('sails').load from 100 seconds to 7 seconds by moving the node_modules directory inside the VM and loading it via the NODE_PATH variable mentioned above.

However, sails hard codes the node_modules path all over the place. Here's an example in upgrade-datastore.js:

      // Before trying to actually require the adapter, make sure we know the real module path:
      var node_modules = path.resolve(sails.config.appPath, 'node_modules');
      var modulePath = path.join(node_modules, moduleName);

      // Then make sure the module exists
      if (!fs.existsSync(modulePath)) {

        // If adapter doesn't exist, log an error and exit
        return Err.fatal.__UnknownAdapter__(connectionObject.adapter, modelID, sails.majorVersion, sails.minorVersion);
      }

This means that, despite node being able to run sails, load deps, etc from my external node_modules file, sails lays an egg when it can't find anything in the local node_modules.

We can get around this by symlinking node_modules back into the application folder, but it's pretty annoying. Would appreciate if the node_modules path was configurable, or at least respected the NODE_PATH environment variable.

I'm still sort of getting up to speed with node.js, but maybe require.resolve will do what you want? http://stackoverflow.com/a/15303236/329700

[sgress454]

Interesting. The issue is that in the places where you see node_modules hard-coded in Sails core, it's because it needs to require a file from the project's set of modules, not from the core's set of modules. So what's really required here is to be able to specify a different working directory for require, so that it uses NODE_PATH as intended when it doesn't find a local node_modules folder. This might be possible by setting process.env.NODE_PATH before calls to require, and resetting it after, since require is synchronous, but it still feels dangerous. This requires some more thought (no pun intended... :P )

[HallM]

I had a similar problem, same root cause really, while trying to figure out a way to bring in Consolidate with ReactJS support without the need for the path modifications. If the local copy of Sails was a copy instead of a symlink, removing all the set paths worked for consolidate. I tested this with just the Consolidate by removing consolidate.js from lib/hooks/views/, modifying the configure to require('consolidate') without the path, then installed Consolidate and any template engine on the end project. By pushing Consolidate to the user project, it should allow Sails to use a specific version of EJS internally while a user can use a different version if they wish. I haven't dug into much code to know where ejs is used to know if that could be pushed upward as well, especially with it in the package.json by default.

Edit: looks like the symlink vs copy has been somewhat discussed before. Adding my thoughts to it though:

• Any user who git/hg/svn clone's a repo is going to pull from npm anyway instead of symlinks, thus only the original creator benefits from the symlinks.
• Makes it easier for different projects on a user's file system to use different versions of Sails
• Removes the need for many of the coded paths
• May lead to a future concept similar to Grunt with a CLI tool that uses the local copy
• While it does remove ability to update /usr/lib/node_modules/sails and have all projects reference it, I feel each project still has to edit the local package.json file to make sure the versions are correct while also performing any necessary updates to their code. In the end, it's not as easy as updating a single directory anyway.
• Can just pull in Consolidate (maybe others?) from npm instead of needing edits while trying to merge changes in constantly.
• Does consume more disk space, but we are only talking MBs per project and disks are too cheap.

[sailsbot]

Thanks for posting, @kevinburke. I'm a repo bot-- nice to meet you!

It has been 60 days since there have been any updates or new comments on this page. If this issue has been resolved, feel free to disregard the rest of this message. On the other hand, if you are still waiting on a patch, please:

• review our contribution guide to make sure this submission meets our criteria (only verified bugs with documented features, please; no questions, commentary, or bug reports about undocumented features or unofficial plugins)
• create a new issue with the latest information, including updated version details with error messages, failing tests, and a link back to the original issue. This allows GitHub to automatically create a back-reference for future visitors arriving from search engines.

Thanks so much for your help!

[nrempel]

Hi @sgress454, I just ran into this issue as well.

I'm working around the issue with a symlink but it still feels like a bit of a code-smell to me.

Can you help me understand why

it needs to require a file from the project's set of modules, not from the core's set of modules

I'm sure I'm missing something obvious here but shouldn't those be treated as the same thing?

Thanks!

[mikermcneil]

@nrempel Thanks for the detailed response. I totally agree. This is the kind of thing that other folks have solved using peer dependencies. The problem with that is that they're not really a proper part of NPM, and it violates NPM's philosophy. Of course, what we're doing isn't much better-- it's just historically been more well-behaved... that is, of course, until NPM 3 came around.

A better solution would probably be to change the way we do dynamic loading throughout Sails. Instead of running require from core, we could expect you to require the appropriate package from your config. This is a little harder to work with for folks new to Sails, because it makes the error/troubleshooting messages harder to read, but I think at this point it's probably the best solution. If anyone reading this has time to open a proposal as a starting point for gathering ideas on how folks would like to see this in Sails v1, I'd appreciate it.

Dynamically-required things to potentially address:

• Waterline adapters
• Connect session adapters
• consolidate view engines
• NPM-installed generators
• NPM-installed hooks
• Skipper adapters
• various grunt-contrib plugins for the default asset pipeline
• Socket.io adapters (this is really just Redis though, and in production, sails-hook-sockets takes advantage of Redis directly to perform some stuff not supported by Socket.io, so this needs to stay the way it is for now. Fortunately, I've never heard of this particular one causing anyone trouble)