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

fs: move fs/promises to fs.promises #20504

Merged
merged 1 commit into from
May 7, 2018
Merged

fs: move fs/promises to fs.promises #20504

merged 1 commit into from
May 7, 2018

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented May 3, 2018

Refs: nodejs/TSC#389

This only deals with moving fs/promises to fs.promises, and nothing about a CLI flag.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label May 3, 2018
@joyeecheung
Copy link
Member

Is it going to be backported to v10? Per the stability index, this is possible, and it does not seem too late while v10.0.0 is just out.

https://nodejs.org/api/documentation.html#documentation_stability_index

Stability: 1 - Experimental This feature is still under active development and subject to non-backwards compatible changes, or even removal, in any future version. Use of the feature is not recommended in production environments. Experimental features are not subject to the Node.js Semantic Versioning model.

@joyeecheung
Copy link
Member

cc @nodejs/tsc

@jasnell
Copy link
Member

jasnell commented May 4, 2018

Yes, it would need to be.

Copy link
Contributor

@MylesBorins MylesBorins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

I would like to see this land before next Tuesday if possible so we can cut a 10.1.0 that includes this ASAP

@cjihrig
Copy link
Contributor Author

cjihrig commented May 5, 2018

@rauschma
Copy link

rauschma commented May 6, 2018

Wouldn’t this make this API less convenient to use with ES modules?

Compare:

import {mkdir} from 'fs/promises';

import * as fs from 'fs';
const {mkdir} = fs.promises;

@joyeecheung
Copy link
Member

joyeecheung commented May 6, 2018

@rauschma This is not about this API, see the discussion in nodejs/TSC#389 . In summary:

  1. We are figuring out a policy for adding new module ids, possibly under @nodejs or @core
  2. We introduced the WHATWG URL as require('url').URL, for consistency the promise API should've been mounted the similar way (or the WHATWG URL should've been mounted as require('url/whatwg') but the ship may have been sailed). Anyway the module id is still subject to change while it is experimental and putting it under an existing module allows us to figure out the namespace issue while the users experiment with the functionality. The module id is not final at this point. The fact that it is experimental means it could change or even disappear (unlikely though) in the future.

PR-URL: nodejs#20504
Refs: nodejs/TSC#389
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
@cjihrig cjihrig merged commit 975f6c1 into nodejs:master May 7, 2018
@cjihrig cjihrig deleted the fs branch May 7, 2018 23:49
@ChALkeR ChALkeR added experimental Issues and PRs related to experimental features. fs Issues and PRs related to the fs subsystem / file system. labels May 8, 2018
MylesBorins pushed a commit that referenced this pull request May 8, 2018
PR-URL: #20504
Refs: nodejs/TSC#389
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
MylesBorins added a commit that referenced this pull request May 8, 2018
Notable Changes:

* console:
  - make console.table() use colored inspect (TSUYUSATO Kitsune)
    #20510
* fs:
  - move fs/promises to fs.promises (cjihrig)
    #20504
* http:
  - added aborted property to request (Robert Nagy)
    #20094
* n-api:
  - initialize a module via a special symbol (Gabriel Schulhof)
    #20161
* src:
  - add public API to expose the main V8 Platform (Allen Yonghuang Wang)
    #20447

PR-URL: Coming Soon
@MylesBorins MylesBorins mentioned this pull request May 8, 2018
MylesBorins pushed a commit that referenced this pull request May 8, 2018
PR-URL: #20504
Refs: nodejs/TSC#389
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
MylesBorins added a commit that referenced this pull request May 8, 2018
Notable Changes:

* console:
  - make console.table() use colored inspect (TSUYUSATO Kitsune)
    #20510
* fs:
  - move fs/promises to fs.promises (cjihrig)
    #20504
* http:
  - added aborted property to request (Robert Nagy)
    #20094
* n-api:
  - initialize a module via a special symbol (Gabriel Schulhof)
    #20161
* src:
  - add public API to expose the main V8 Platform (Allen Yonghuang Wang)
    #20447

PR-URL: #20606
@EvanCarroll
Copy link
Contributor

Using 10.9, this isn't working for me at all,

This is all that's in my .js,

import fs from 'fs';

When I try to run node --experimental-modules --loader ./test.js

> (node:3278) ExperimentalWarning: The ESM module loader is experimental.
(node:3278) UnhandledPromiseRejectionWarning: /home/ecarroll/code/pg-url/sql/test.js:1
(function (exports, require, module, __filename, __dirname) { import fs from 'fs';
                                                                     ^^

SyntaxError: Unexpected identifier
    at new Script (vm.js:73:7)
    at createScript (vm.js:245:10)
    at Proxy.runInThisContext (vm.js:297:10)
    at Module._compile (internal/modules/cjs/loader.js:657:28)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
    at Module.load (internal/modules/cjs/loader.js:599:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
    at Function.Module._load (internal/modules/cjs/loader.js:530:3)
    at createDynamicModule (internal/modules/esm/translators.js:56:15)
    at setExecutor (internal/modules/esm/create_dynamic_module.js:50:23)
(node:3278) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:3278) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

@devsnek
Copy link
Member

devsnek commented Aug 28, 2018

@EvanCarroll rename test.js to test.mjs

kellyselden added a commit to kellyselden/node that referenced this pull request Jul 16, 2020
According to nodejs#20504, this API was removed.
@KonradLinkowski
Copy link

Maybe you should change it in the documentation https://nodejs.org/api/fs.html#fs_promise_example

@devsnek
Copy link
Member

devsnek commented Sep 22, 2020

@KonradLinkowski fs/promises was added back at a later date.

@KonradLinkowski
Copy link

KonradLinkowski commented Sep 22, 2020

@devsnek So something may be wrong with github actions, because I was getting Error: Cannot find module 'fs/promises' error until I used fs.promises.

@ljharb
Copy link
Member

ljharb commented Sep 22, 2020

@KonradLinkowski it depends what node version you're using. node v10.0.0 or >= v14.0 are required for fs/promises to work.

@KonradLinkowski
Copy link

@ljharb Oh ok, I was pretty sure that node12 is the latest version, but as you can see I was very wrong xd
Thanks for help :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experimental Issues and PRs related to experimental features. fs Issues and PRs related to the fs subsystem / file system. lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.