support-sinon.js breaking builds for older versions of node.js

[robinfhu]

The new support-sinon.js post install script breaks build, because it uses the "supports-color" module, which uses ES6 syntax, which requires a new version of node.js.

We are on an older version of node.js.

• Sinon version : 4.4.8
• Environment : node v0.10.22
• Example URL :
• Other libraries you are using: karma-sinon-chai

node scripts/support-sinon.js


node_modules/sinon/node_modules/supports-color/index.js:2
error	21-Mar-2018 11:27:27	const os = require('os');
error	21-Mar-2018 11:27:27	^^^^^
error	21-Mar-2018 11:27:27	SyntaxError: Use of const in strict mode.
error	21-Mar-2018 11:27:27	    at Module._compile (module.js:439:25)
error	21-Mar-2018 11:27:27	    at Object.Module._extensions..js (module.js:474:10)
error	21-Mar-2018 11:27:27	    at Module.load (module.js:356:32)
error	21-Mar-2018 11:27:27	    at Function.Module._load (module.js:312:12)
error	21-Mar-2018 11:27:27	    at Module.require (module.js:364:17)
error	21-Mar-2018 11:27:27	    at require (module.js:380:17)
error	21-Mar-2018 11:27:27	    at Object.<anonymous> (node_modules/sinon/lib/sinon/color.js:3:21)
error	21-Mar-2018 11:27:27	    at Module._compile (module.js:456:26)
error	21-Mar-2018 11:27:27	    at Object.Module._extensions..js (module.js:474:10)
error	21-Mar-2018 11:27:27	    at Module.load (module.js:356:32)

[fatso83]

I am thinking that if you use a five year old Node version then staying on the latest version of Sinon can't be that big of a deal :-)

In any case, this was unforeseen, but I am not sure support for such old engines is anything else but coincidental. On the other hand, we have striven to be ES5 pure in the codebase, so bundling ES2015 libraries seems a bit wrong.

What do you think, @mroderick?

[fatso83]

I do see one point of fixing this though, which might have been obvious to everyone except myself. This doesn't happen by running tests, but by installing a devDependency. That means tests might be run on Node 9, but @robinfhu might strive to keep his OWN code backwards compatible. This prevents that. That speaks for fixing this. We could revert the change in the meantime, Morgan?

[mroderick]

I am in favour of fixing this.

While we have written that we only want to support Node LTS versions, we're actually staying quite compatible with old Node.

It seems silly that we would break compatibility for old Node versions, just for the post-install script. If we're going to break it, then at least let's get more substantial benefit (i.e. benefit for majority of users).

Do you want to create PR to revert it?

[fatso83]

I could also just revert to version 3.2.3? It was version 4 that introduced ES2015.

And if we do decide to keep install compatibility with Node 0.10, shouldn't we explicitly test for it in our Travis config? Whatever the resolution, I could add Node 0.10 or v3 there just to get a early warning.

[mroderick]

I've been thinking about this some more today, and decided to do some investigation.

We've been using supports-color since 527086a, we started out using v4.4.0.

527086a became sinon@4.0.2, that we released on 2017-10-25.

This means that at least since 2017-10-25, Sinon has been partially broken in node@0.10.

The fact that Sinon is now dependent on supports-color on install has just made it quite obvious that some things are broken in node@0.10.

Considering that no one has noticed this for nearly six months, suggests to me that Sinon's user base has moved on from using node@0.10.

I don't think we should go out of our way to support legacy node versions by testing for them in Travis.

I do think we should fix the postinstall script to not fail in unsupported runtimes.
In the same PR, I think we should also update the postinstall script to detect unsupported runtime and say the runtime is unsupported by Sinon and that mileage might vary.

[robinfhu]

Thanks for taking the time to consider this. For now, we have fixed sinon to v4.4.6 in our package.json file.

[mlucool]

I completely get why you would want something that you know some people see, but it has interfered with things that parse output. I am seeing the use of the postinstall step growing in a large number of libraries. To help with this (especially in CI), what do you think about something like putting NODE_NO_SUPPORT_MESSAGE=true in your environment to opt out? Since this is explicit, you know people will have at least seen your plea for help.

Thanks for the consideration!

[fatso83]

@mlucool I think that would work for me, for just those reasons. But you would still get :

> sinon@4.4.8 postinstall /home/carlerik/dev/sinon
> node scripts/support-sinon.js

even if this flag suppressed the output from the command, though it achieves your intended purpose if you do npm run postinstall -s