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.

Sign up for GitHub

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

Jump to bottom

feature-request - add [lazy] option parents to fs.writeFile[Sync], fs.appendFile[Sync], fs.open[Sync] #35775

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

feature-request - add [lazy] option parents to fs.writeFile[Sync], fs.appendFile[Sync], fs.open[Sync] #35775

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
87f66f2
fs: add option `parents` to
kaizhu256 Jan 12, 2021
eacc195
- update function fs.openSync to use hasNoEntryError() instead of try…
kaizhu256 Jan 19, 2021
a78c1e2
Update lib/fs.js
kaizhu256 Jan 19, 2021
528331a
Update lib/internal/fs/promises.js
kaizhu256 Jan 19, 2021
480eaf7
If parents is not set, please avoid creating this closure in the firs…
kaizhu256 Jan 19, 2021
ae1a6cf
If parents is not set, please avoid creating this closure in the firs…
kaizhu256 Jan 19, 2021
b006217
Update lib/fs.js
kaizhu256 Jan 19, 2021
9129c42
Update lib/fs.js
kaizhu256 Jan 19, 2021
b967e5c
resolve conflicts with branch master
kaizhu256 Aug 4, 2021
43c2b6f
revert doc/api/fs.md to branch-master.
kaizhu256 Aug 4, 2021
a132adb
Merge branch 'master' into alpha
kaizhu256 Aug 4, 2021
ca8deb8
Update doc/api/fs.md with `parents` option.
kaizhu256 Aug 4, 2021
7b5b3e1
flag to flags shim:
kaizhu256 Aug 4, 2021
47dbc40
test: fix failing tests
kaizhu256 Aug 4, 2021
8bd103b
fs: fix eslint errors
kaizhu256 Aug 4, 2021
229c4e4
Apply suggestions from code review
kaizhu256 Aug 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 103 additions & 12 deletions doc/api/fs.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1354,6 +1354,9 @@ try {
<!-- YAML
added: v0.6.7
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/35775
description: add option `parents`.
- version: v10.0.0
pr-url: https://github.com/nodejs/node/pull/12562
description: The `callback` parameter is no longer optional. Not passing
Expand All @@ -1376,6 +1379,8 @@ changes:
* `encoding` {string|null} **Default:** `'utf8'`
* `mode` {integer} **Default:** `0o666`
* `flag` {string} See [support of file system `flags`][]. **Default:** `'a'`.
* `parents` {boolean} create parent-directories if they do not exist.
**Default:** `false`.
* `callback` {Function}
* `err` {Error}

Expand Down Expand Up @@ -1415,6 +1420,9 @@ fs.open('message.txt', 'a', (err, fd) => {
<!-- YAML
added: v0.6.7
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/35775
description: add option `parents`.
- version: v7.0.0
pr-url: https://github.com/nodejs/node/pull/7831
description: The passed `options` object will never be modified.
Expand All @@ -1429,6 +1437,8 @@ changes:
* `encoding` {string|null} **Default:** `'utf8'`
* `mode` {integer} **Default:** `0o666`
* `flag` {string} See [support of file system `flags`][]. **Default:** `'a'`.
* `parents` {boolean} create parent-directories if they do not exist.
**Default:** `false`.

Synchronously append data to a file, creating the file if it does not yet
exist. `data` can be a string or a [`Buffer`][].
Expand Down Expand Up @@ -2788,13 +2798,13 @@ this API: [`fs.mkdtemp()`][].
The optional `options` argument can be a string specifying an encoding, or an
object with an `encoding` property specifying the character encoding to use.

## `fs.open(path[, flags[, mode]], callback)`
## `fs.open(path[, flag[, mode]], callback)`
<!-- YAML
added: v0.0.2
changes:
- version: v11.1.0
pr-url: https://github.com/nodejs/node/pull/23767
description: The `flags` argument is now optional and defaults to `'r'`.
description: The `flag` argument is now optional and defaults to `'r'`.
- version: v9.9.0
pr-url: https://github.com/nodejs/node/pull/18801
description: The `as` and `as+` flags are supported now.
Expand All @@ -2805,7 +2815,7 @@ changes:
-->

* `path` {string|Buffer|URL}
* `flags` {string|number} See [support of file system `flags`][].
* `flag` {string|number} See [support of file system `flags`][].
Copy link
Member

Choose a reason for hiding this comment

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

why was this changed from flags to flag?

Copy link
Contributor

Choose a reason for hiding this comment

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

To be fair, the FS API and documentation already uses those inconsistently (E.G.: flags in fs.openSync, flag in fs.readFileSync).

Copy link
Contributor

Choose a reason for hiding this comment

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

Note that changing the documentation from flags to flag would break the existing hypertext links (from StackOverflow, blog posts, etc.), which is something we'd rather not do.

**Default:** `'r'`.
* `mode` {string|integer} **Default:** `0o666` (readable and writable)
* `callback` {Function}
Expand All @@ -2828,6 +2838,28 @@ a colon, Node.js will open a file system stream, as described by
Functions based on `fs.open()` exhibit this behavior as well:
`fs.writeFile()`, `fs.readFile()`, etc.

## `fs.open(path[, options], callback)`
<!-- YAML
added: REPLACEME
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/35775
description: add option `parents`.
-->

* `path` {string|Buffer|URL}
* `options` {Object|string}
* `flag` {string|number} See [support of file system `flags`][].
**Default:** `'r'`.
* `mode` {string|integer} **Default:** `0o666` (readable and writable)
* `parents` {boolean} create parent-directories if they do not exist.
**Default:** `false`.
* `callback` {Function}
* `err` {Error}
* `fd` {integer}

Asynchronous file open using `options` signature-form.

## `fs.opendir(path[, options], callback)`
<!-- YAML
added: v12.12.0
Expand Down Expand Up @@ -2884,13 +2916,13 @@ and cleaning up the directory.
The `encoding` option sets the encoding for the `path` while opening the
directory and subsequent read operations.

## `fs.openSync(path[, flags, mode])`
## `fs.openSync(path[, flag, mode])`
<!-- YAML
added: v0.1.21
changes:
- version: v11.1.0
pr-url: https://github.com/nodejs/node/pull/23767
description: The `flags` argument is now optional and defaults to `'r'`.
description: The `flag` argument is now optional and defaults to `'r'`.
- version: v9.9.0
pr-url: https://github.com/nodejs/node/pull/18801
description: The `as` and `as+` flags are supported now.
Expand All @@ -2901,7 +2933,7 @@ changes:
-->

* `path` {string|Buffer|URL}
* `flags` {string|number} **Default:** `'r'`.
* `flag` {string|number} **Default:** `'r'`.
Copy link
Member

Choose a reason for hiding this comment

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

why was this changed from flags to flag?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why was this changed from flags to flag?

  1. i think its lesser evil to alternative -- internal monkey-patching/future-tech-debt converting flag => flags, when called by fs.writeFile[Sync] and fs.appendFile[Sync]
fs.writeFileSync("aa/bb/cc/dd.txt", {
    // do we really want internal monkey-patching from
    // `flag` => `flags` when passed to fs.openSync()?
    flag: "w+",
    parents: true
});

  1. now is the time to change it without backward-compatibility issues.

  2. flags appears in fs.createReadStream(path,{flags}) and fs.createWriteStream(path,{flags}) but those api's aren't as tightly coupled with fs.writeFile[Sync] and fs.appendFile[Sync]

if ppl feel strongly about it, i can revert back to flags and add monkey-patching to play nice with fs.writeFile[Sync] and fs.appendFile[Sync].

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be reverted from this PR as it's not related with the parents option. Maybe you can open another PR focusing on this change? I agree the lack of consistency is kinda bad here, and if we decide to keep the inconsistency, we should at least discuss it thoroughly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

k, reverted documentation of fs.open[Sync]() and fs.promises.open() back to flags in commit ca8deb8.

See [support of file system `flags`][].
* `mode` {string|integer} **Default:** `0o666`
* Returns: {number}
Expand All @@ -2911,6 +2943,25 @@ Returns an integer representing the file descriptor.
For detailed information, see the documentation of the asynchronous version of
this API: [`fs.open()`][].

## `fs.openSync(path[, options])`
<!-- YAML
added: REPLACEME
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/35775
description: add option `parents`.
-->

* `path` {string|Buffer|URL}
* `options` {Object|string}
* `flag` {string|number} See [support of file system `flags`][].
**Default:** `'r'`.
* `mode` {string|integer} **Default:** `0o666` (readable and writable)
* `parents` {boolean} create parent-directories if they do not exist.
**Default:** `false`.

Synchronous file open using `options` signature-form.

## `fs.read(fd, buffer, offset, length, position, callback)`
<!-- YAML
added: v0.0.2
Expand Down Expand Up @@ -4415,6 +4466,9 @@ details.
<!-- YAML
added: v0.1.29
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/35775
description: add option `parents`.
- version: v15.2.0
pr-url: https://github.com/nodejs/node/pull/35993
description: The options argument may include an AbortSignal to abort an
Expand Down Expand Up @@ -4453,6 +4507,8 @@ changes:
* `encoding` {string|null} **Default:** `'utf8'`
* `mode` {integer} **Default:** `0o666`
* `flag` {string} See [support of file system `flags`][]. **Default:** `'w'`.
* `parents` {boolean} create parent-directories if they do not exist.
**Default:** `false`.
* `signal` {AbortSignal} allows aborting an in-progress writeFile
* `callback` {Function}
* `err` {Error}
Expand Down Expand Up @@ -4537,6 +4593,9 @@ to contain only `', World'`.
<!-- YAML
added: v0.1.29
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/35775
description: add option `parents`.
- version: v14.12.0
pr-url: https://github.com/nodejs/node/pull/34993
description: The `data` parameter will stringify an object with an
Expand All @@ -4563,6 +4622,8 @@ changes:
* `encoding` {string|null} **Default:** `'utf8'`
* `mode` {integer} **Default:** `0o666`
* `flag` {string} See [support of file system `flags`][]. **Default:** `'w'`.
* `parents` {boolean} create parent-directories if they do not exist.
**Default:** `false`.

Returns `undefined`.

Expand Down Expand Up @@ -5193,6 +5254,10 @@ the error raised if the file is not accessible.
### `fsPromises.appendFile(path, data[, options])`
<!-- YAML
added: v10.0.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/35775
description: add option `parents`.
-->

* `path` {string|Buffer|URL|FileHandle} filename or `FileHandle`
Expand All @@ -5201,6 +5266,8 @@ added: v10.0.0
* `encoding` {string|null} **Default:** `'utf8'`
* `mode` {integer} **Default:** `0o666`
* `flag` {string} See [support of file system `flags`][]. **Default:** `'a'`.
* `parents` {boolean} create parent-directories if they do not exist.
**Default:** `false`.
* Returns: {Promise}

Asynchronously append data to a file, creating the file if it does not yet
Expand Down Expand Up @@ -5421,17 +5488,17 @@ characters directly to the `prefix` string. For instance, given a directory
`prefix` must end with a trailing platform-specific path separator
(`require('path').sep`).

### `fsPromises.open(path, flags[, mode])`
### `fsPromises.open(path, flag[, mode])`
<!-- YAML
added: v10.0.0
changes:
- version: v11.1.0
pr-url: https://github.com/nodejs/node/pull/23767
description: The `flags` argument is now optional and defaults to `'r'`.
description: The `flag` argument is now optional and defaults to `'r'`.
-->

* `path` {string|Buffer|URL}
* `flags` {string|number} See [support of file system `flags`][].
* `flag` {string|number} See [support of file system `flags`][].
**Default:** `'r'`.
* `mode` {string|integer} **Default:** `0o666` (readable and writable)
* Returns: {Promise}
Expand All @@ -5447,6 +5514,25 @@ by [Naming Files, Paths, and Namespaces][]. Under NTFS, if the filename contains
a colon, Node.js will open a file system stream, as described by
[this MSDN page][MSDN-Using-Streams].

### `fsPromises.open(path[, options])`
<!-- YAML
added: REPLACEME
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/35775
description: add option `parents`.
-->

* `path` {string|Buffer|URL}
* `options` {Object|string}
* `flag` {string|number} See [support of file system `flags`][].
**Default:** `'r'`.
* `mode` {string|integer} **Default:** `0o666` (readable and writable)
* `parents` {boolean} create parent-directories if they do not exist.
**Default:** `false`.

Asynchronous file open that returns a `Promise` using `options` signature-form.

### `fsPromises.opendir(path[, options])`
<!-- YAML
added: v12.12.0
Expand Down Expand Up @@ -5780,6 +5866,9 @@ The `atime` and `mtime` arguments follow these rules:
<!-- YAML
added: v10.0.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/35775
description: add option `parents`.
- version: v15.2.0
pr-url: https://github.com/nodejs/node/pull/35993
description: The options argument may include an AbortSignal to abort an
Expand All @@ -5795,11 +5884,13 @@ changes:
-->

* `file` {string|Buffer|URL|FileHandle} filename or `FileHandle`
* `data` {string|Buffer|Uint8Array|Object}
* `data` {string|Buffer|TypedArray|DataView|Object}
* `options` {Object|string}
* `encoding` {string|null} **Default:** `'utf8'`
* `mode` {integer} **Default:** `0o666`
* `flag` {string} See [support of file system `flags`][]. **Default:** `'w'`.
* `parents` {boolean} create parent-directories if they do not exist.
**Default:** `false`.
* `signal` {AbortSignal} allows aborting an in-progress writeFile
* Returns: {Promise}

Expand Down Expand Up @@ -6255,7 +6346,7 @@ the file contents.
[`fs.lutimes()`]: #fs_fs_lutimes_path_atime_mtime_callback
[`fs.mkdir()`]: #fs_fs_mkdir_path_options_callback
[`fs.mkdtemp()`]: #fs_fs_mkdtemp_prefix_options_callback
[`fs.open()`]: #fs_fs_open_path_flags_mode_callback
[`fs.open()`]: #fs_fs_open_path_flag_mode_callback
[`fs.opendir()`]: #fs_fs_opendir_path_options_callback
[`fs.opendirSync()`]: #fs_fs_opendirsync_path_options
[`fs.read()`]: #fs_fs_read_fd_buffer_offset_length_position_callback
Expand All @@ -6274,7 +6365,7 @@ the file contents.
[`fs.write(fd, string...)`]: #fs_fs_write_fd_string_position_encoding_callback
[`fs.writeFile()`]: #fs_fs_writefile_file_data_options_callback
[`fs.writev()`]: #fs_fs_writev_fd_buffers_position_callback
[`fsPromises.open()`]: #fs_fspromises_open_path_flags_mode
[`fsPromises.open()`]: #fs_fspromises_open_path_flag_mode
[`fsPromises.opendir()`]: #fs_fspromises_opendir_path_options
[`fsPromises.utimes()`]: #fs_fspromises_utimes_path_atime_mtime
[`inotify(7)`]: https://man7.org/linux/man-pages/man7/inotify.7.html
Expand Down
2 changes: 1 addition & 1 deletion doc/api/worker_threads.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1140,7 +1140,7 @@ active handle in the event system. If the worker is already `unref()`ed calling
[`cluster` module]: cluster.md
[`data:` URL]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/Data_URIs
[`fs.close()`]: fs.md#fs_fs_close_fd_callback
[`fs.open()`]: fs.md#fs_fs_open_path_flags_mode_callback
[`fs.open()`]: fs.md#fs_fs_open_path_flag_mode_callback
[`markAsUntransferable()`]: #worker_threads_worker_markasuntransferable_object
[`perf_hooks.performance`]: perf_hooks.md#perf_hooks_perf_hooks_performance
[`perf_hooks` `eventLoopUtilization()`]: perf_hooks.md#perf_hooks_performance_eventlooputilization_utilization1_utilization2
Expand Down
Loading