WIP Upgrade all dependencies

curran commented 5 years ago

While I was in here bumping magic-string, I figured I'd try upgrading all dependencies to the latest.

 os-homedir                   ^1.0.1  →   ^2.0.0 
 eslint                      ^4.19.1  →  ^5.13.0 
 rollup                       0.66.0  →    1.1.2 
 rollup-plugin-commonjs       =9.1.8  →   =9.2.0 
 rollup-plugin-node-resolve   ^3.4.0  →   ^4.0.0 
 source-map                   ^0.6.1  →   ^0.7.3 
 acorn                        ^6.0.5  →  ^6.1.0

This has broken the tests, but only in a few places (localized only to modules.js and sourcemaps). I'm not able to investigate this further, but thought I'd share these results if anyone wants to pick this up and follow it through.

adrianheine commented 5 years ago

If you want to you could retry on top of 77ab2365788f7d2f0f05f79e2925e98b259404e0.

curran commented 5 years ago

@adrianheine Thanks! I did merge from there, and now it's only down to 3 failing tests, localized to sourcemaps.

curran commented 5 years ago

@adrianheine I found the cause of the failing tests and changed them to pass.

It was a breaking change in the source-map API that made a constructor asynchronous. See https://github.com/mozilla/source-map#new-sourcemapconsumerrawsourcemap

adrianheine commented 5 years ago

That source-map version switches to WebAssembly, too, though, which would break node@4 compatibility. Same for the rollup bump. I'm willing to start dropping node@4 support once Debian Buster is released, which should happen in the next months.

mourner commented 5 years ago

I'm willing to start dropping node@4 support once Debian Buster is released

Can you elaborate on why Node version decisions should be based on the releases schedule of Debian? Node v4 was declared "End of Life" in April 2018, meaning no updates, even for critical security vulnerabilities, for more than a year. This means continuing to use Node v4 is a significant security risk and should be actively discouraged.

nathancahill commented 5 years ago

I second @mourner. Node 4 is dead weight at this point. Debian Buster should be released in 4 days on July 6. Should we make some of these moves before then and have a release ready to go?

adrianheine commented 5 years ago

I'm generally sympathetic to dropping node 4 support soon, but I don't understand the hurry.

mourner commented 5 years ago

@adrianheine here are a few reasons:

As a potential contributor of Buble, I don't even want to install Node 4 (e.g. to make sure it works) because it's been completely dead for 15 months, has long-time known unpatched security vulnerabilities and is a risk to run. So are many other contributors.
To support this ancient version that no one uses anywhere, we have to ban many dependency upgrades (since these have long moved on), stripping us of potential performance improvements and important bug fixes.
You're the only person I know in the JS community who still uses Node v4.

adrianheine commented 5 years ago

I don't expect you to install node 4, that's what travis is for, and I still don't see a real, shown advantage of upgrading these dependencies. However, I agree that the two curves of people being bothered by having to support node 4 (going upwards) and the people using bublé on node 4 (going downwards if not already at 0) probably have met some time ago, so I'm willing to merge this once it's in good shape.

adrianheine commented 5 years ago

(I still think it's generally a nice thing to do to support older nodejs versions that are somewhat used if that doesn't block the project substantially. Pinning a few dependencies to perfectly fine versions doesn't count as substantially blocking in my opinion. As for why supporting nodejs 4: I expect some people stuck on the nodejs version packaged in Debian stable on their servers or hosted environments. So, to sum it up: I still think it was the right choice to support nodejs 4 for some time, longer than most other projects do, and I think it's the right thing to weigh contributor's comfort and maintainability higher than nodejs 4 support at this point.)

mourner commented 5 years ago

@adrianheine this is very reasonable, and I'm not strongly against keeping the deps pinned to older versions, since they're simple enough and only used for development now. It will only be a problem when direct deps move on, — I just think it's best to anticipate this proactively because it might be harder to upgrade later, and generally it's a good practice to keep dependencies upgraded periodically because you can run into very obscure, hard-to-debug bugs later (e.g. when on a newer Node) because of some incompatibility with an old dependency — I've been bitten by this before, although in larger projects. Moreover, given Buble's purpose, it's typically run as a dev tool locally rather than on a server or a hosted environment.

I don't expect you to install node 4, that's what travis is for

Upgrading a dependency and then waiting to see if it will fail on Travis is not an ideal development workflow though...

mourner commented 4 years ago

Superceded by #250

bublejs / buble

WIP Upgrade all dependencies #185