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

Drop NodeJs 8 support & Require NodeJs >= 10 #3818

Closed
tomap opened this issue Oct 29, 2019 · 14 comments · Fixed by #4255
Closed

Drop NodeJs 8 support & Require NodeJs >= 10 #3818

tomap opened this issue Oct 29, 2019 · 14 comments · Fixed by #4255

Comments

@tomap
Copy link
Contributor

tomap commented Oct 29, 2019

This issue is very similar to #3508
We will need to drop NodeJs 8 Support as it reached EOL on December 31st
See https://github.com/nodejs/Release#release-schedule

It will also bring the benefit to be abe to use more native APIs.
Here is a list of issues/PR pending NodeJs 8 drop:

Note that in most cases, it will imply a major version bump
We also will need to check which version to use (10.0, 10.1, ...?)

@SukkaW
Copy link
Member

SukkaW commented Oct 29, 2019

hexojs/hexo-util#114

After Node.js 8 is dropped. we can mitigate decodeURL(str) to newest whatwg url api, and remove const { URL } = require('url'); from mitigated helper.

@SukkaW
Copy link
Member

SukkaW commented Oct 29, 2019

Also, according to the performance comparison by https://blog.kuzzle.io/bluebird-vs-native-vs-async/await-state-of-promises-performances-in-2019 , async / await looks promising on Node.js 10 / 12. We might start to implement #3328 after Nodejs.8 is dropped.

image
image

@curbengh
Copy link
Contributor

curbengh commented Nov 2, 2019

Much like our previous dropping of Node 6, since we're developing solely on master branch (not version branch), we could only drop Node 8 (in the master branch, not yet in npm) when we're sure that there will be no more minor version bump (i.e. 4.x).

My idea is that we can work on anything that doesn't break Node 8 for now (I can't think of any Node 10+ only feature, that is relevant to this repo), even if it requires some minor workaround (e.g. hexojs/hexo-util#117); those workarounds can be removed later once we drop Node 8.

As in timeline, if we're sure there'll be no more 4.x, let say in December, we can even start dropping Node 8 at that time (again, just in repo, not npm). But that's assuming we're not in 🎅 mood...

Since Node 8 is quite modern, as in it even supports some ES2019 features, I don't expect the transition Node 8 -> 10 to be as drastic as Node 6 -> 8, hopefully it's smoother this time.

We also will need to check which version to use (10.0, 10.1, ...?)

A brief check on https://node.green/ shows some features supported in v10.17 but not older; some not supported in v10.3.

Our previous approach was pretty conservative (set v8.6 as minimum even though v8.16 was available), but I suggest this time to set to the latest minor version available. This is to minimize the need to bump again during minor cycle, i.e. to avoid https://github.com/hexojs/hexo/pull/3778/files#diff-b9cfc7f2cdf78a7f4b91a753d10865a2R81. There is a possibility that we might use v10.17+ features.

@curbengh
Copy link
Contributor

Pinning this.

@curbengh curbengh pinned this issue Dec 17, 2019
@curbengh curbengh added this to the 5.0.0 milestone Dec 19, 2019
@curbengh
Copy link
Contributor

Just stumbled upon a Node 8-only bug, hexojs/hexo-fs#50

@curbengh curbengh unpinned this issue Dec 23, 2019
@tomap
Copy link
Contributor Author

tomap commented Dec 29, 2019

Hi,
I just recheck https://node.green/
and the table is a bit misleading,but what I understand is that there is no difference between all the versions that are above v10.9 +
so the table says 10.18, but when you hover this version, you'll see that it includes 10.9 to 10.18 (and probably 10.19 when it will be out)

@curbengh
Copy link
Contributor

curbengh commented Dec 30, 2019

(and probably 10.19 when it will be out)

that's not guaranteed though, 10.19 could introduce a new feature just like 10.8 -> 10.9 or 10.3 -> 10.4.

10.18 includes a high-severity security fix for the npm cli (release note, vulnerability details). Unless user manually updates the npm, I suggest to require at least 10.18 (for now). I know it's not strictly our business, I just feel uneasy not alerting users about it (especially when given the choice).

But it's still early to say, since we still got plenty of stuff to do for v5. For now, we can keep monitoring node.green for 10.19+.

@SukkaW
Copy link
Member

SukkaW commented Dec 30, 2019

@tomap @curbengh

Node.js 10 will start its maintenance LTS at Apr. 1st, 2020. We could choose the first Node.js 10 Maintenance LTS version then.

@curbengh
Copy link
Contributor

curbengh commented Jan 4, 2020

+1; I understand it's a huge milestone, but hopefully we can release v5 before that 🤞

@SukkaW
Copy link
Member

SukkaW commented Apr 25, 2020

Since hexojs/hexo-fs#55 bring up the usage of Node.js 10.12 API, we could bump our minimum required Node.js version to 10.13 (which is the first LTS version of Node.js 10). Any idea?

cc @hexojs/core

@stevenjoezhang
Copy link
Member

Some test cases failed in Node.js v10.20.1. Need to investigate.

1019 passing (11s)
  4 failing

  1) Hexo
       Box
         Box
           process() - skip (mtime changed but hash matched):
     AssertError: expected spy to be called with match 
[_File] {
  params: {  },
  path: "a.txt",
  source: "/Users/StevenJoeZhang/hexo-plugin/hexo/test/scripts/box/box_tmp/test/a.txt",
  type: "update"
} { path: "a.txt", type: "skip" } 
      at Object.fail (/Users/StevenJoeZhang/hexo-plugin/hexo/node_modules/sinon/lib/sinon/assert.js:107:21)
      at failAssertion (/Users/StevenJoeZhang/hexo-plugin/hexo/node_modules/sinon/lib/sinon/assert.js:66:16)
      at Object.assert.(anonymous function) [as calledWithMatch] (/Users/StevenJoeZhang/hexo-plugin/hexo/node_modules/sinon/lib/sinon/assert.js:92:13)
      at Context.it (/Users/StevenJoeZhang/hexo-plugin/hexo/test/scripts/box/box.js:160:17)

  2) Hexo
       Box
         Box
           watch() - update:
     AssertError: expected spy([_File] {
  params: {  },
  path: "a.txt",
  source: "/Users/StevenJoeZhang/hexo-plugin/hexo/test/scripts/box/box_tmp/test/a.txt",
  type: "skip"
}) at Hexo.tryCatcher (/Users/StevenJoeZhang/hexo-plugin/hexo/node_modules/bluebird/js/release/util.js:16:23) to be called with match 
      at Object.fail (/Users/StevenJoeZhang/hexo-plugin/hexo/node_modules/sinon/lib/sinon/assert.js:107:21)
      at failAssertion (/Users/StevenJoeZhang/hexo-plugin/hexo/node_modules/sinon/lib/sinon/assert.js:66:16)
      at Object.assert.(anonymous function) [as calledWithMatch] (/Users/StevenJoeZhang/hexo-plugin/hexo/node_modules/sinon/lib/sinon/assert.js:92:13)
      at Context.it (/Users/StevenJoeZhang/hexo-plugin/hexo/test/scripts/box/box.js:385:17)

  3) Hexo
       Box
         Box
           watch() - update with simple "ignore" option:

      AssertionError: expected { Object (source, path, ...) } to have deep property 'type' of 'update', but got 'skip'
      + expected - actual

      -skip
      +update
      
      at Context.it (/Users/StevenJoeZhang/hexo-plugin/hexo/test/scripts/box/box.js:528:22)

  4) Hexo
       Console
         generate
           update theme source files:

      AssertionError: expected 'b' to deeply equal 'bb'
      + expected - actual

      -b
      +bb
      
      at Context.it (/Users/StevenJoeZhang/hexo-plugin/hexo/test/scripts/console/generate.js:207:22)

@SukkaW SukkaW reopened this Jun 7, 2020
@SukkaW
Copy link
Member

SukkaW commented Jun 7, 2020

We'd better leave this issue opened.

Although most of the satellite repo has update the CI configuration, but the minimum required Node.js version in those package.json haven't been update yet. We could track them here.

@curbengh
Copy link
Contributor

I don't think we need to apply to all (official) plugins, some deps of those plugins hardly have any update.

@SukkaW
Copy link
Member

SukkaW commented Jul 25, 2020

@curbengh LGTM. It is ok to close the issue now.

@SukkaW SukkaW closed this as completed Jul 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants