Skip to content
This repository was archived by the owner on Mar 10, 2020. It is now read-only.

build: change min node version to 8.3.0 #996

Merged
merged 1 commit into from
May 15, 2019

Conversation

michaelsbradleyjr
Copy link
Contributor

@michaelsbradleyjr michaelsbradleyjr commented May 14, 2019

Also adjust Travis CI config to perform builds and tests with node 8.

npm install, npm run build, and npm run test all succeeded locally with node version 8.3.0.

The reason for version 8.3.0 instead of 8.0.0 is that 8.3.0 introduced support for object rest/spread, which is used by at least one of this package's devDependencies (go-ipfs-dep, in production install of that devDep). It may be the case that at runtime older versions of node would work correctly with the built package, but it seems simpler to settle on the minimum supported version being a version that can build, test, and use the package.

Closes #983.

Also adjust Travis CI config to perform builds and tests with node 8.

`npm install`, `npm run build`, and `npm run test` all succeeded locally with
node version `8.3.0`.

The reason for version `8.3.0` instead of `8.0.0` is that `8.3.0` introduced
support for object rest/spread, which is used by at least one of this package's
devDependencies (`go-ipfs-dep`). It may be the case that at runtime older
versions of node would work correctly with the built package, but it seems
simpler to settle on the minimum supported version being a version that can
build, test, and use the package.

Closes ipfs-inactive#983.
@michaelsbradleyjr
Copy link
Contributor Author

michaelsbradleyjr commented May 14, 2019

I'm not sure how to adjust the Travis CI config so that tests are run with node version 8 on Windows.

I'm also not sure, if this PR is to be merged, whether it should include changes to the README indicating that the Maintenance LTS of node is also supported.

@alanshaw alanshaw merged commit 7a47e3b into ipfs-inactive:master May 15, 2019
@filips123
Copy link
Contributor

@alanshaw @michaelsbradleyjr There is again problem with support for Node.js 8. It now fails because of peer-id which requires Node.js 10. See my CI test for details.

@michaelsbradleyjr
Copy link
Contributor Author

michaelsbradleyjr commented Jul 12, 2019

@filips123, indeed. The problem is especially apparent when you use yarn instead of npm in a project that has ipfs-http-client as a dependency because yarn always enforces "engines" at any depth in the dependency tree whereas npm does not.

So, in hindsight, this PR was actually mostly pointless and should probably be reverted (@alanshaw).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should the required NodeJS version be >=8.0.0?
3 participants