Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ava errors on Node <4.5.0 due to hullabaloo-config-manager #1354

Closed
exogen opened this issue Apr 10, 2017 · 16 comments
Closed

ava errors on Node <4.5.0 due to hullabaloo-config-manager #1354

exogen opened this issue Apr 10, 2017 · 16 comments
Labels
bug current functionality does not work as desired help wanted

Comments

@exogen
Copy link

exogen commented Apr 10, 2017

Description

Running ava errors immediately on Node 4.4 and lower with TypeError: this is not a typed array from hullabaloo-config-manager. That module's package.json specifies its minimum Node version as 4.5 and they ain't kiddin' – this affects basic usage of AVA.

(Unfortunately npm doesn't seem to warn about engines for transitive dependencies, so it doesn't even warn about hullabaloo-config-manager, but I believe it would warn about ava directly.)

I recommend updating AVA's engines from Node >=4 to >=4.5.

Test Source

// Empty test is fine to reproduce.

Error Message & Stack Trace

 ⠋ /Users/becbrian/Projects/Tests/test-ava/node_modules/ava/lib/cli.js:196
					throw err;
					^

TypeError: this is not a typed array.
    at Function.from (native)
    at Verifier.toBuffer (/Users/becbrian/Projects/Tests/test-ava/node_modules/hullabaloo-config-manager/lib/Verifier.js:137:19)
    at /Users/becbrian/Projects/Tests/test-ava/node_modules/ava/lib/babel-config.js:78:46
From previous event:
    at Api.run (/Users/becbrian/Projects/Tests/test-ava/node_modules/ava/api.js:71:5)
    at Object.exports.run.api.on.api.run.then [as run] (/Users/becbrian/Projects/Tests/test-ava/node_modules/ava/lib/cli.js:187:7)
    at Object.<anonymous> (/Users/becbrian/Projects/Tests/test-ava/node_modules/ava/cli.js:22:24)
    at Module._compile (module.js:409:26)
    at Object.Module._extensions..js (module.js:416:10)
    at Module.load (module.js:343:32)
    at Function.Module._load (module.js:300:12)
    at Function.Module.runMain (module.js:441:10)
    at startup (node.js:139:18)
    at node.js:968:3

Config

(no config necessary)

Command-Line Arguments

ava

Relevant Links

Environment

$ node -e "var os=require('os');console.log('Node.js ' + process.version + '\n' + os.platform() + ' ' + os.release())"
Node.js v4.4.7
darwin 16.5.0
$ ava --version
0.19.1
$ npm --version
4.4.4
@novemberborn
Copy link
Member

Ah yeah. Buffer.from() was backported in 4.5. Since new Buffer() is deprecated in newer Node.js versions it's annoying to have to work around it. We should update the version range and see if that leads to any issues. There have been security releases since 4.5 even so ideally nobody is using such old versions in production.

@novemberborn novemberborn added bug current functionality does not work as desired help wanted labels Apr 11, 2017
@exogen
Copy link
Author

exogen commented Apr 12, 2017

There have been security releases since 4.5 even so ideally nobody is using such old versions in production.

I agree. Unfortunately pre-4.5 seems to be the default on CircleCI, which is how I discovered this (all my projects using AVA were failing). I added a circle.yml to force a newer version. AVA can't fix that directly, but debugging would have been quicker if npm had warned about engines (it doesn't warn about hullabaloo-config-manager, but would warn about ava since it's a direct dep). I can open a PR to update the version range this evening. :)

@novemberborn
Copy link
Member

I can open a PR to update the version range

That'd be great!

debugging would have been quicker if npm had warned about engines

Do you think AVA should flat out refuse to start if it detects an outdated Node.js version?

@exogen
Copy link
Author

exogen commented Apr 13, 2017

Do you think AVA should flat out refuse to start if it detects an outdated Node.js version?

I'd probably give people's environment the benefit of the doubt (like maybe they patched it up/polyfilled it to work somehow?), but print a warning when AVA starts – this would be more immediately visible than npm's warning (the user may have installed a long time ago before running ava in non-CI cases).

@novemberborn
Copy link
Member

I'd probably give people's environment the benefit of the doubt (like maybe they patched it up/polyfilled it to work somehow?), but print a warning when AVA starts – this would be more immediately visible than npm's warning (the user may have installed a long time ago before running ava in non-CI cases).

👍

@southpolesteve
Copy link

@novemberborn Just came across this bug too. Wanted to chime in and say that AWS Lambda is still uses 4.3.2 as their "Node 4" version. We're working on getting stuff over to node 6 but it was only released recently. I would imagine plenty of Lambda users still using 4.3.2 in production including ourselves.

@novemberborn
Copy link
Member

@southpolesteve interesting. I still think the right answer is to require 4.5 or above though. Even if this particular dependency is under our (mine) control, it's quite likely (over Node.js 4's maintenance lifetime) for other dependencies to assume the newer Buffer APIs exist.

@southpolesteve
Copy link

Yeah I think you are probably right. Just sucks that this happened at all. I've also sent a note to AWS telling them to upgrade their node 4 version :)

@reconbot
Copy link

Node 4 is lts you know? I think https://github.com/feross/safe-buffer would do what's needed here.

@novemberborn
Copy link
Member

@reconbot yes we could use that in dependencies under our control (e.g. that team members maintain) but I reckon we'll see other dependencies assume Node.js 4.5. It's a losing game.

@reconbot
Copy link

reconbot commented May 24, 2017

I think it's a worthwhile game as long as node4 is in LTS (until 2018-04-01) there are a lot of people still on node 4.3.2 because of lambda (and probably older because of Linux distros) and it's worth trying to support them if we can. I've submitted a patch to hullabaloo-config-manager novemberborn/hullabaloo-config-manager#15

LTS isn't forever

@novemberborn
Copy link
Member

LTS is only meaningful though if you actually update. There have been several security releases since 4.5.

@reconbot
Copy link

Lambda is a major holdout, I did accidentally get the program manger in touch with someone from Node who works on LTS, but I doubt we'll see much movement there for a while. I found out that only 4.2+ is considered LTS which still lower that 4.5. Since you do control it and it's the only package blocking <4.5 do you mind holding out a little longer?

@novemberborn
Copy link
Member

@avajs/core thoughts?

@sindresorhus
Copy link
Member

@novemberborn I would just add safe-buffer. I agree it's dumb, but not worth wasting time on issues about this.

@novemberborn
Copy link
Member

@sindresorhus fair enough. @reconbot I'll try and get to your PR on the weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug current functionality does not work as desired help wanted
Projects
None yet
Development

No branches or pull requests

5 participants