Js-ipfs: Not fully compatible with node 4 anymore

Created on 8 Mar 2017 · 14Comments · Source: ipfs/js-ipfs

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.

Source

dignifiedquire

All 14 comments

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

daviddias on 8 Mar 2017

cc @SilentCicero

kumavis on 8 Mar 2017

drop Node.js 4 support :)

thisconnect on 9 Mar 2017

👍1

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.

victorb on 9 Mar 2017

👍1

@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

dignifiedquire on 9 Mar 2017

@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.

victorb on 9 Mar 2017

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.

daviddias on 13 Mar 2017

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.

dignifiedquire on 13 Mar 2017

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 on 14 Mar 2017

👍1

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

kumavis on 17 Mar 2017

@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 on 17 Mar 2017

@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

> [email protected] pretest /Users/dignifiedquire/opensource/ethjs-util
> npm run lint


> [email protected] lint /Users/dignifiedquire/opensource/ethjs-util
> npm run lint:js


> [email protected] lint:js /Users/dignifiedquire/opensource/ethjs-util
> npm run lint:eslint -- .


> [email protected] lint:eslint /Users/dignifiedquire/opensource/ethjs-util
> eslint --ignore-path .gitignore --ignore-pattern **/**.min.js "."


> [email protected] 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.

dignifiedquire on 17 Mar 2017

👍1

Node.js 4 has reached its maintenance period on April 2017 and now we are at half of its way of its maintenance phase. Node.js 6 adoption has been incredible and surpassed Node.js 4 installations by 100K in the end of 2016.