Restore support down to node 0.8

[ljharb]

I downgraded module-deps to v4 and no tests broke, so the v5 upgrade seems entirely unnecessary (#1743, #1785). It's also possible that the fix for #1782 simply failed to include a regression test; however, with module-deps v4, the backticks test passes just fine.

If that's incorrect, please help by adding test cases that necessitate the upgrade, and I'll work on restoring older node support to module-deps and its dep tree first (I'll do that next, regardless).

I also enabled branch protection on master, so we won't be able to merge anything in the future that fails tests, unrelated to this PR.

[bcomnes]

I think we should restore 0.12 support to module-deps, and then leave the bump here. The point of the update was to not crash when spread operators were present in code in efforts to track node's support for the newer language features. Perhaps we should add a test with the spread operator to browserify as well.

[ljharb]

@bcomnes that's totally fine! If there's a test case that fails on module-deps < 5, then absolutely I wouldn't make that fail by downgrading module-deps :-)

If there's not a test in browserify for it, then it doesn't matter if it's broken in browserify :-)

[bcomnes]

Can we hold off from downgrading module deps until tomorrow or Sunday when we can restore 0.12 support in module-deps and dependents when we can just keep it? One of the key aspects of the major was to not crash on files with spread. Agreed that not testing the spread operator in browserify was an oversight for this release.

[ljharb]

I certainly don't plan to merge this hastily; since you've told me that module-deps v5 fixes an untested bug, I'll wait til we have a test for it for sure. It wouldn't be very sensible to want to support older nodes and then risk breaking newer ones :-D

If you're otherwise OK with this PR, then great! I'll work on module-deps and its tree next.

[ljharb]

detective supporting older node is blocked by adrianheine/acorn5-object-spread#1 (or an equivalent fix). I've asked the maintainer about it.

[bcomnes]

Capturing some points from IRC:

I forked acorn5-object-spread and made it support 0.8 again, and released updates to detective and soon to release one for module-deps that will roll support back. I think once thats in, browserify will run on 0.8 again and we can avoid having to downgrade module deps (probably just pin it at 5.0.1 when it comes out.)

I think it would be beneficial at some point to determine guidelines about when the projects in the org can start dropping support for EOL runtimes though.

[bcomnes]

For example, if we work on support way back to 0.8, we will inevitably start falling behind on our dependencies which will be more likely to drop support before browserify.

One not so important example is not getting to use tap 11 in module-deps now because it doesn't run in node < iojs. Not critical, but its illustrative of the situation we will sign up for if we dont drop support at some point.

[bcomnes]

I just pinned module deps to ^5.0.1 to run tests. Its a semver patch, but we can pin it here as well.

[ljharb]

Everything's passed tests, and the npm incident is over :-)

This should be good to go!

[TehShrike]

I don't personally believe that supporting end-of-life versions of node.js provides enough value to justify adding any amount of complexity to the codebase, even this small amount in tests.

If there was great value in browserify supporting unsupported platforms, then this pull request looks good to me. I'm not sure I can bring myself to hit the "approve" button now though :-x

If the rest of the community really values this, though, I won't be annoying about it.

[bcomnes]

I'm okay with doing this for now, but in general think it would be completely fair and would like to drop support for node <= 4. We've discussed some of the reasons in IRC and I think its fine to take this in until we can get an agreement on what to do. I think @substack making a call would be helpful here. We can make that change in a different PR though since this work is already completed.

[bcomnes]

Thought about it more, we should merge and minor release this, make note in the changelog and wait for the next scenario to figure it out. Maybe it will be more obvious what to do by then.

[ljharb]

I think that would be great. When it becomes truly untenable to maintain support for something, i will happily write the pr that drops it, but until then, I’d very much like to see us put forth trivial effort to retain it.

[goto-bus-stop]

also happy to take this. i expect we'll have similar situations come up with different acorn plugins when V8 lands more new syntax features unflagged tho.

[adrianheine]

Hi, current maintainer of https://github.com/adrianheine/acorn5-object-spread here. I don't think it's reasonable to support Nodejs versions < 4. They have been unsupported for over a year, Debian stable and oldstable-backports ship 4.8.2, your dependencies will most likely drop support for them (as @bcomnes pointed out) before you do. You shouldn't break support on purpose, but putting effort into it imho is a disservice to your users.

However, acorn currently supports those ancient engines, so once the proposal reaches stage 4 we will probably merge rest/spread properties support that also runs on them. In general, https://www.npmjs.com/acorn-stage3 and my other acorn plugins don't support Nodejs < 4 and I'm so far not convinced rewriting them in such a way is a good idea.

[bcomnes]

I'm going to merge and minor release this at lunch today (someone feel free to beat me to it). With semver, the fixes should already be in browserify though for the time being.

I think merging this for now is fine for the following reasons:

• We still get the 15 release features.
• We did't discuss all concerns held by stakeholders prior to explicitly dropping support for < 4. We should have sorted this out before.
• We should explicitly decide on engine support.
• Acorn still is ES5 still and will eventually get features once they reach stage 0 (or something)

Reasons why I'm against this in general (capturing this)

• Supporting language features of node LTS/maintenance through stable in browserify is more important than maintaining support for node runtimes nobody has any business running. Optimizing for this is important.
• Acorn tracks TC-39 schedules when it comes to language support, and not node support schedules which runs a bit faster when it comes to feature adoption. Browserify's entire premise is running node modules in the browser (vs webpack, the generic and highly flexible js/asset bundler), so tracking TC-39 schedule instead of node core's schedule is a mistake. I think it would be more beneficial to the project and vast majority of users to optimize for language support for node LTS-Stable including the ability to use acorn plugins directly.
• We shouldn't be helping people run versions of node they shouldn't run. That energy would be better spent helping these people update their runtime. I still don't see a rational situation someone is running node less than 4 and can't upgrade to 4, let alone running less than 4 and trying to use browserify bins from 2018 to make bundles. Libs that depend on browserify as a dep that are still targeting EOL node should pin their deps to the version that works across their engine range. Browserify dependents that support EOL arn't a reason for browserify itself aim at EOL support.
• We shouldn't drop support unless we need to, but if its out of our platforms support window, and the primary goals of this project would benefit from the work of prominent acorn contributors at the cost of dropping EOL support, we shouldn't have any hesitations to do so. Even if acorn remains 0.8 compatible for the next 5 years, clearly the plugin ecosystem and contributors are not and maintaining language support parity with stable is more important and not worth the energy to support things people aught not do.
• Semver major works. If you can't update node to 4 or higher (but again, why not update? you are running EOL), run a version of your deps that work. They are not going anywhere.

[ljharb]

Thank you!

[bcomnes]

browserify@15.1.0