Not fully compatible with node 4 anymore

[dignifiedquire]

While adding a yarn.lock file I discovered that a dependency we are using is actually only working on node 6: https://github.com/ethjs/ethjs-util. As specified in its engine field. I also checked and it actually fails its tests on node 4.

yarn why ethjs-util
yarn why v0.21.3
[1/4] 🤔  Why do we have the module "ethjs-util"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
info This module exists because "ipld-resolver#ipld-eth-block#ethereumjs-block#ethereumjs-util" depends on it.
info Disk size without dependencies: "276kB"
info Disk size with unique dependencies: "372kB"
info Disk size with transitive dependencies: "372kB"
info Amount of shared dependencies: 2
✨  Done in 3.05s.

[daviddias]

@kumavis @wanderer can that tool become Node.js 4 compatible?

[kumavis]

cc @SilentCicero

[thisconnect]

drop Node.js 4 support :)

• http://node.green
• https://github.com/nodejs/LTS

[victorb]

drop Node.js 4 support :)

Not sure that's a great idea, yet. According to the last user survey from NodeJS Foundation found that most people were running version 4 in 2016. Probably that looks different today, but couldn't find anything from 2017. https://nodejs.org/static/documents/2016-survey-report.pdf ||

Also, version 4 is still actively in LTS and would be too soon to drop it. https://github.com/nodejs/LTS#lts-schedule

The only thing I can spot (from a quick skim through it) in https://github.com/ethjs/ethjs-util that would make it incompatible is template strings, which should fairly easy to fix if we agree version 4 support is important.

[dignifiedquire]

@victorbjelkholm that is not the only issue. It seems it relies on the new way of ascii and utf8 character code point handling, as some of the tests fail on the encoding methods on node@4. So it's more involved to make it work on node@4. Also template strings are available in node@4

[victorb]

@dignifiedquire thanks for clarifying, you're right. Did not run the tests myself but just read through quickly.

Still, if enough people are still on version 4, it would be a bad idea to stop supporting it, even if it would take a bit of work.

[daviddias]

We've a couple of options in this case.

• Update ethjs-util to support Node.js 4 for now
• Remove eth support by default on js-ipfs. Support can be added to a running node with ipldResolver.support.add
• Drop Node.js 4

The first option would be the optimal, the second makes me sad, the third is something that was planned, but not 'now', we might reconsider next month during the gather.

[dignifiedquire]

Given the comments above, I don't think it's likely Node 4 support will be merged. So I would suggest to remove eth support for the time being and discuss and decide how to move forward about Node 4 next month at the gathering.

[kumavis]

i think ethereumjs-block can be updated to remove ethereumjs-util and thus ethjs-util
but i have not investigated. may be a bit of a tangle to remove it.

[kumavis]

It seems it relies on the new way of ascii and utf8 character code point handling, as some of the tests fail on the encoding methods on node@4.

@dignifiedquire we are discussing this now - where can we find the test failures

[dignifiedquire]

@kumavis I have a branch on my laptop but never pushed it, I can push probably in an hour or so. As far as I remember I just installed deps on node 4 and ran the test

[dignifiedquire]

@kumavis https://github.com/dignifiedquire/ethjs-util/tree/node-4 this branch fails for me:

nvm use 4
Now using node v4.4.7 (npm v3.10.6)
➜  ethjs-util (master) ✗ npm test

> ethjs-util@0.1.4 pretest /Users/dignifiedquire/opensource/ethjs-util
> npm run lint


> ethjs-util@0.1.4 lint /Users/dignifiedquire/opensource/ethjs-util
> npm run lint:js


> ethjs-util@0.1.4 lint:js /Users/dignifiedquire/opensource/ethjs-util
> npm run lint:eslint -- .


> ethjs-util@0.1.4 lint:eslint /Users/dignifiedquire/opensource/ethjs-util
> eslint --ignore-path .gitignore --ignore-pattern **/**.min.js "."


> ethjs-util@0.1.4 test /Users/dignifiedquire/opensource/ethjs-util
> mocha ./src/tests/**/*.js -R spec --timeout 2000000



  check all exports
    ✓ should have all exports available
    ✓ should convert intToHex
    ✓ should throw when invalid abi
    ✓ should detect invalid length hex string
    ✓ should stripHexPrefix strip prefix of valid strings
    ✓ should stripHexPrefix strip prefix of mix hexed strings
    ✓ should stripHexPrefix bypass if not string
    ✓ valid padToEven should pad to even
    ✓ valid padToEven should pad to even check string prefix 0
    ✓ should padToEven throw as expected string got null
    ✓ should padToEven throw as expected string got undefined
    ✓ should padToEven throw as expected string got {}
    ✓ should padToEven throw as expected string got new Buffer()
    ✓ should padToEven throw as expected string got number
    ✓ method getKeys should throw as expected array for params got number
    ✓ method invalid getKeys with allow empty and no defined value
    ✓ method valid getKeys with allow empty and false
    ✓ method getKeys should throw as expected array for params got number
    ✓ method getKeys should throw as expected array for params got object
    ✓ method getKeys should throw as expected array for params got null
    ✓ method getKeys should throw as expected array for params got false
    ✓ valid getKeys should get keys from object in array
    ✓ valid isHexString tests
    ✓ invalid isHexString tests
    ✓ valid arrayContainsArray should array contain every array
    ✓ valid getBinarySize should get binary size of string
    ✓ invalid getBinarySize should throw invalid type Boolean
    ✓ invalid getBinarySize should throw invalid type object
    ✓ invalid getBinarySize should throw invalid type Array
    ✓ valid arrayContainsArray should array some every array
    ✓ method arrayContainsArray should throw as expected array for params got false
    ✓ method arrayContainsArray should throw as expected array for params got false
    ✓ method arrayContainsArray should throw as expected array for params got {}
    fromAscii
      1) should turn myString to 0x6d79537472696e67
      2) should turn myString to 0x6d79537472696e6700
      3) should turn 5èÆÕL]|�ξ�7«�2(Ð�Y
<e!ßd/ñõì
         :z¦Î¦±ç·÷Í¢Ëß6*��ñ�ùC1ÉUÀé2Ó�B� to 0x0300000035e8c6d54c5d127c9dcebe9e1a37ab9b05321128d097590a3c100000000000006521df642ff1f5ec0c3a7aa6cea6b1e7b7f7cda2cbdf07362a85088e97f19ef94331c955c0e9321ad386428c
    fromUtf8
      ✓ should turn myString to 0x6d79537472696e67
      ✓ should turn myString to 0x6d79537472696e67
      ✓ should turn expected value to 0x65787065637465642076616c7565
    toUtf8
      ✓ should turn 0x6d79537472696e67 to myString
      ✓ should turn 0x6d79537472696e6700 to myString
      ✓ should turn 0x65787065637465642076616c7565000000000000000000000000000000000000 to expected value
    toAsciiTests
      4) should turn 0x6d79537472696e67 to myString
      5) should turn 0x6d79537472696e6700 to myString
      6) should turn 0x0300000035e8c6d54c5d127c9dcebe9e1a37ab9b05321128d097590a3c100000000000006521df642ff1f5ec0c3a7aa6cea6b1e7b7f7cda2cbdf07362a85088e97f19ef94331c955c0e9321ad386428c to 5èÆÕL]|�ξ�7«�2(Ð�Y
<e!ßd/ñõì
         :z¦Î¦±ç·÷Í¢Ëß6*��ñ�ùC1ÉUÀé2Ó�B�
    intToHex
      ✓ should convert a int to hex
    intToBuffer
      ✓ should convert a int to a buffer
    hex prefix
      ✓ should add
      ✓ `should return on non-string input


  43 passing (44ms)
  6 failing

  1) check all exports fromAscii should turn myString to 0x6d79537472696e67 :

      AssertionError: expected '0x6d6d6d6d6d6d6d6d' to equal '0x6d79537472696e67'
      + expected - actual

      -0x6d6d6d6d6d6d6d6d
      +0x6d79537472696e67

      at Function.assert.strictEqual (node_modules/chai/lib/chai/interface/assert.js:178:32)
      at Context.<anonymous> (src/tests/test.index.js:314:16)

  2) check all exports fromAscii should turn myString to 0x6d79537472696e6700 :

      AssertionError: expected '0x6d6d6d6d6d6d6d6d6d' to equal '0x6d79537472696e6700'
      + expected - actual

      -0x6d6d6d6d6d6d6d6d6d
      +0x6d79537472696e6700

      at Function.assert.strictEqual (node_modules/chai/lib/chai/interface/assert.js:178:32)
      at Context.<anonymous> (src/tests/test.index.js:314:16)

  3) check all exports fromAscii should turn 5èÆÕL]|�ξ�7«�2(Ð�Y
<e!ßd/ñõì
         :z¦Î¦±ç·÷Í¢Ëß6*��ñ�ùC1ÉUÀé2Ó�B� to 0x0300000035e8c6d54c5d127c9dcebe9e1a37ab9b05321128d097590a3c100000000000006521df642ff1f5ec0c3a7aa6cea6b1e7b7f7cda2cbdf07362a85088e97f19ef94331c955c0e9321ad386428c :

      AssertionError: expected '0x0303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303' to equal '0x0300000035e8c6d54c5d127c9dcebe9e1a37ab9b05321128d097590a3c100000000000006521df642ff1f5ec0c3a7aa6cea6b1e7b7f7cda2cbdf07362a85088e97f19ef94331c955c0e9321ad386428c'
      + expected - actual

      -0x0303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303
      +0x0300000035e8c6d54c5d127c9dcebe9e1a37ab9b05321128d097590a3c100000000000006521df642ff1f5ec0c3a7aa6cea6b1e7b7f7cda2cbdf07362a85088e97f19ef94331c955c0e9321ad386428c

      at Function.assert.strictEqual (node_modules/chai/lib/chai/interface/assert.js:178:32)
      at Context.<anonymous> (src/tests/test.index.js:314:16)

  4) check all exports toAsciiTests should turn 0x6d79537472696e67 to myString :

      AssertionError: expected 'mmmmmmmm' to equal 'myString'
      + expected - actual

      -mmmmmmmm
      +myString

      at Function.assert.strictEqual (node_modules/chai/lib/chai/interface/assert.js:178:32)
      at Context.<anonymous> (src/tests/test.index.js:357:16)

  5) check all exports toAsciiTests should turn 0x6d79537472696e6700 to myString :

      AssertionError: expected 'mmmmmmmmm' to equal 'myString\u0000'
      + expected - actual

      -mmmmmmmmm
      +myString

      at Function.assert.strictEqual (node_modules/chai/lib/chai/interface/assert.js:178:32)
      at Context.<anonymous> (src/tests/test.index.js:357:16)

  6) check all exports toAsciiTests should turn 0x0300000035e8c6d54c5d127c9dcebe9e1a37ab9b05321128d097590a3c100000000000006521df642ff1f5ec0c3a7aa6cea6b1e7b7f7cda2cbdf07362a85088e97f19ef94331c955c0e9321ad386428c to 5èÆÕL]|�ξ�7«�2(Ð�Y
<e!ßd/ñõì
         :z¦Î¦±ç·÷Í¢Ëß6*��ñ�ùC1ÉUÀé2Ó�B� :

      AssertionError: expected '\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003\u0003' to equal '\u0003\u0000\u0000\u00005èÆÕL]\u0012|�ξ�\u001a7«�\u00052\u0011(Ð�Y\n<\u0010\u0000\u0000\u0000\u0000\u0000\u0000e!ßd/ñõì\f:z¦Î¦±ç·÷Í¢Ëß\u00076*�\b��ñ�ùC1ÉUÀé2\u001aÓ�B�'
      + expected - actual

      -
      +5èÆÕL]|�ξ�7«�2(Ð�Y
      +<e!ßd/ñõì
                :z¦Î¦±ç·÷Í¢Ëß6*��ñ�ùC1ÉUÀé2Ó�B�

      at Function.assert.strictEqual (node_modules/chai/lib/chai/interface/assert.js:178:32)
      at Context.<anonymous> (src/tests/test.index.js:357:16)



npm ERR! Test failed.  See above for more details.