Updated dependencies

[apowers313]

Updated all possible dependencies based on the packages that were reported out of date by david-dm.org. Not all the latest packages are compatible with sails, so after updating there are five packages that are still out of date: async 1.4.2, ejs 2.3.3, express 4.13.3, grunt-contrib-less 1.0.1, and lodash 3.10.1.

This PR passes all tests, although there is an exec() call in appHelper.js:build() that errored out because my path has a space in it -- this PR includes a patch for that bug as well.

I didn't look into the root cause of the five packages that break the tests -- I am hoping that updating these packages de-obfuscates the actual broken dependencies so that someone more familiar with the code can fix those errors (and maybe we can get rid of that annoying red "dependencies out of date" badge ;).

[kevinburkeshyp]

Updated all possible dependencies based on the packages that were reported out of date by david-dm.org.

Maybe not the right place for this, but I'm not sure that "keeping all dependencies up to date" is a great goal to shoot for. Dependencies often introduce breaking changes, these can be extremely subtle. It's also not clear of the benefit from keeping your dependencies up to date; if only the dependencies change, you're not getting any new features. Sure, if there is a security hotfix then update but it doesn't seem like this is for that purpose.

[kevinburkeshyp]

(The best possible solution is to limit the number of things you depend on, but that doesn't seem too popular of an opinion in the JS community.)

[apowers313]

@kevinburkeshyp are you volunteering to look through the list of updated packages to see which have security or bug fixes that we want to pull in? :)

[kevinburkeshyp]

No, I'm saying if it's not broken and you're not trying to get new features or security fixes, trying to fix it will, in the best case, do nothing, and in the worst case, break the application.

[apowers313]

Yea, I get what you are saying. The flip-side is that if nobody is monitoring those packages for security holes or bugs, then it forces development into a reactive position to solve critical issues after they arise in deployments. Best case scenario is that nothing happens, worst case scenario is that lots of production sails implementations assume they are safe up until the day that they are egregiously violated on a large scale. The other consideration is that by not keeping up to date with packages, the project will start incurring code debt that will make future upgrades more costly than the incremental maintenance of keeping up to date.

I think there are pros and cons on both sides, and the answer probably boils down to philosophy more than facts. I'll leave it to the maintainers as to whether their philosophy is that this PR is useful as a whole, has some useful pieces, or belongs in the great bit-bucket in the sky.

[kevinburkeshyp]

The "if it aint broke don't fix it" aphorism is made irrelevant by semver. You get to choose which version series to subscribe to.

At Twilio we had a deployment tool which was broken by a change from a dependency which went from x.y.9 to x.y.10. The change prevented proper fleet rollbacks, meaning that broken nodes stayed in the load balancer and served traffic for much longer than they should have.

As a JS library maintainer you can't really enforce guarantees about which interfaces people are using. I'm also skeptical that people are appropriately applying semver in every instance, or can predict how their library is being used in every instance, or have the appropriate test coverage to be able to determine whether a change breaks things. Sails has 50 dependencies.

[apowers313]

@tjwebb yep, node-uuid was removed as a dependency; expect.js and benchmark were removed as devDependencies.

Merged the current master with the updated dependencies and all tests pass.

[sylvainlap]

and what about lodash ?
Updating it to 3.10.1 introduces breaking changes ?

[apowers313]

@sylvainlap :

Not all the latest packages are compatible with sails, so after updating there are five packages that are still out of date: async 1.4.2, ejs 2.3.3, express 4.13.3, grunt-contrib-less 1.0.1, and lodash 3.10.1.

If I remember correctly, lodash causes an exception to be thrown before sails is even raised, which isn't surprising given all the places it is used and the fact that sails is behind by a full major version.

[apowers313]

Update: the first error occurs due to _.clone() not accepting functions anymore when sails tries to clone an emitter. I'm not sure if this is a recurring theme throughout the sails code.

[sgress454]

As maintainers of Sails--and I'm speaking for both the core team and the greater Sails community--it's our responsibility to keep the framework stable while moving it forward. Hopefully the maintainers of all of these packages we depend on feel the same way. However, we can't just blindly trust that something labeled "stable" or "latest" is automatically safe for us to use. Our automated tests are a good line of defense, but they're not--and will never be--perfect, and they're somewhat less useful when we can't also immediately see the code that's changed.

I'm currently going through each of our dependencies by hand, inspecting their changelogs (or commit histories if no changelogs exist), updating individually and both running our automated tests and testing against a robust Sails app. I've already identified a couple of cases where upgrading a Grunt dependency causes failures in production mode for apps with a lot of assets, and in some of those cases I'll actually be downgrading the dependency we rely on. We can argue about Grunt in another thread; my point is that we can't just upgrade everything that David says is outdated and call it a day.

Once I'm done thoroughly vetting the dependencies, we'll be locking the versions of everything except those packages we have direct control over (things like anchor, waterline, and sails-hook-sockets). As new versions of locked dependencies come out, we can make patch releases to Sails to upgrade them. Even this approach isn't 100% foolproof, as the dependencies themselves may use semver ranges in their package.json file. But it's at least more stable than the situation now, where from one Sails install to the next you don't know exactly what you're going to get.

Then I'ma drink some eggnog. 🍸