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

Node.js 10 #551

Closed
RyanZim opened this issue Feb 6, 2018 · 10 comments
Closed

Node.js 10 #551

RyanZim opened this issue Feb 6, 2018 · 10 comments
Milestone

Comments

@RyanZim
Copy link
Collaborator

RyanZim commented Feb 6, 2018

Node.js 10 isn't out yet, but I just wanted to post this here so we're aware: https://twitter.com/jasnell/status/960733696461438976

It seems that fs.promise.readFile() etc. will have native promise support. I don't see a lot needing changed on our end, other than exposing this API when it's available.

Can close this when all the maintainers have seen this.

@RyanZim RyanZim added the future label Feb 6, 2018
@manidlou
Copy link
Collaborator

manidlou commented Feb 6, 2018

Got it!

@RyanZim
Copy link
Collaborator Author

RyanZim commented Feb 6, 2018

According to https://github.com/nodejs/Release; Node 10 is slated for release in April, which is also the time for the EOL of Node 4 LTS. I'm thinking we should probably cut one major release containing 10 support and dropping 4 support.

@RyanZim
Copy link
Collaborator Author

RyanZim commented Apr 3, 2018

fs.promise has been moved to fs/promise, so you'd do require('fs/promise') to get the promise API. (See nodejs/node#18777 for the change.) The purpose of this is to be better prepared for ESM.

Now, the question on our end is this: do we have to also support require('fs-extra/promise')? I'm not sure.

  • Could argue yes, we're a drop-in replacement for fs.
  • Could argue no, we support promises already in normal fs-extra, there's no need for fs/promise here.

@jprichardson @manidlou @JPeer264 Thoughts?

@JPeer264
Copy link
Collaborator

JPeer264 commented Apr 3, 2018

I would argue yes, as this library should be a drop-in replacement for fs.

Also in the readme we kinda promise: You don't ever need to include the original fs module again. So if we do not support require(fs-extra/promise) then the previous argument seems a bit falsy.

@RyanZim
Copy link
Collaborator Author

RyanZim commented Apr 3, 2018

@JPeer264 Understood; the counterpoint is that we already have promise support, so you'll never need to use fs/promise.

@JPeer264
Copy link
Collaborator

JPeer264 commented Apr 4, 2018

Hm that's true. After lot of thinking now I think it might be better to keep it as is, but we should mention somewhere that require(fs-extra/promise) is not needed.

@manidlou
Copy link
Collaborator

manidlou commented Apr 6, 2018

yeah.. no need to add it!

@RyanZim
Copy link
Collaborator Author

RyanZim commented Apr 6, 2018

I have gotten clarity from the Node.js core team, and unflagged ESM (ES6 modules) support will not be landing in Node 10.0.0, though there's a possibility it'll land later on the 10.x line. So we don't need to worry about that for now.

@bajtos
Copy link
Contributor

bajtos commented May 9, 2018

When using fs-extra on Node.js 10.1.0, an ExperimentalWarning is printed to console - see #577

@RyanZim
Copy link
Collaborator Author

RyanZim commented May 9, 2018

Closing since this is no longer needed.

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

No branches or pull requests

4 participants