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: runtime deprecate closing FileHandle on garbage collection #28396

Closed
wants to merge 4 commits into from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Jun 23, 2019

Closing the FileHandle on garbage collection is a bad practice.
Runtime deprecate and indicate that an error will be thrown in
the future.

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

@jasnell jasnell added fs Issues and PRs related to the fs subsystem / file system. semver-major PRs that contain breaking changes and should be released in the next major version. promises Issues and PRs related to ECMAScript promises. deprecations Issues and PRs related to deprecations. labels Jun 23, 2019
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Jun 23, 2019
Copy link
Contributor

@cjihrig cjihrig left a comment

Choose a reason for hiding this comment

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

LGTM. The code example in the deprecation docs could probably just be dropped, but it's fine either way.

doc/api/deprecations.md Outdated Show resolved Hide resolved
@addaleax
Copy link
Member

Hm, no strong opinion here, but using a deprecation warning means it’s only shown once rather than for each fd, right? Is that a good thing or should the separate warning be kept (e.g. the way it’s done for unhandled rejections)?

doc/api/deprecations.md Outdated Show resolved Hide resolved
@jasnell
Copy link
Member Author

jasnell commented Jun 25, 2019

@addaleax... Sure, I could keep the other warning.

@jasnell
Copy link
Member Author

jasnell commented Jun 25, 2019

@addaleax ... added back the other warning, fixed a couple linting issues, and updated the test. PTAL :-)

@Trott @lpinca @cjihrig @tniessen ... may I ask y'all to take another look as well?

doc/api/deprecations.md Outdated Show resolved Hide resolved
@jasnell
Copy link
Member Author

jasnell commented Oct 16, 2019

FYI... I'll be getting back to this very soon.

test/parallel/test-fs-promises-file-handle-close.js Outdated Show resolved Hide resolved
test/parallel/test-fs-promises-file-handle-close.js Outdated Show resolved Hide resolved
doc/api/deprecations.md Outdated Show resolved Hide resolved
doc/api/deprecations.md Outdated Show resolved Hide resolved
doc/api/deprecations.md Outdated Show resolved Hide resolved
@fhinkel
Copy link
Member

fhinkel commented Nov 1, 2019

ping @jasnell

@jasnell
Copy link
Member Author

jasnell commented Nov 1, 2019 via email

@jasnell
Copy link
Member Author

jasnell commented Feb 3, 2020

Ping @addaleax @Trott @lpinca @cjihrig @tniessen ... please take another look. Updated this to avoid using process state

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@jasnell
Copy link
Member Author

jasnell commented Feb 4, 2020

CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/2259/ (link may not be immediately activated)

@jasnell jasnell added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 4, 2020
@addaleax addaleax added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Feb 4, 2020
@jasnell
Copy link
Member Author

jasnell commented Feb 5, 2020

CITGM results show no relevant failures.

jasnell added a commit that referenced this pull request Feb 5, 2020
Closing the FileHandle on garbage collection is a bad practice.
Runtime deprecate and indicate that an error will be thrown in
the future.

PR-URL: #28396
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@jasnell
Copy link
Member Author

jasnell commented Feb 5, 2020

Landed in #28396

@jasnell jasnell closed this Feb 5, 2020
cjihrig added a commit to cjihrig/node that referenced this pull request Feb 9, 2020
PR-URL: nodejs#31674
Refs: nodejs#28396
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
BethGriggs added a commit that referenced this pull request Apr 14, 2020
Notable changes:

* (SEMVER-MAJOR) crypto: move pbkdf2 without digest to EOL
  (James M Snell) [#31166](#31166)
* (SEMVER-MAJOR) fs: deprecate closing FileHandle on garbage collection
  (James M Snell) [#28396](#28396)
* (SEMVER-MAJOR) http: move OutboundMessage.prototype.flush to EOL
  (James M Snell) [#31164](#31164)
* (SEMVER-MAJOR) lib: move GLOBAL and root aliases to EOL
  (James M Snell) [#31167](#31167)
* (SEMVER-MAJOR) os: move tmpDir() to EOL
  (James M Snell) [#31169](#31169)
* (SEMVER-MAJOR) src: remove deprecated wasm type check
  (Clemens Backes) [#32116](#32116)
* (SEMVER-MAJOR) stream: move \_writableState.buffer to EOL
  (James M Snell) [#31165](#31165)
* (SEMVER-MINOR) doc: deprecate process.mainModule
  (Antoine du HAMEL) [#32232](#32232)
* (SEMVER-MINOR) doc: deprecate process.umask() with no arguments
  (Colin Ihrig) [#32499](#32499)

* (SEMVER-MAJOR) build: update macos deployment target to 10.13 for 14.x
  (AshCripps) [#32454](#32454)
* (SEMVER-MAJOR) win: block running on EOL Windows versions (João Reis)
  [#31954](#31954)

* (SEMVER-MAJOR) deps: update V8 to 8.1.307.20 (Matheus Marchini)
  [#32116](#32116)

PR-URL: #32181
BethGriggs added a commit that referenced this pull request Apr 15, 2020
Notable changes:

* (SEMVER-MAJOR) crypto: move pbkdf2 without digest to EOL
  (James M Snell) [#31166](#31166)
* (SEMVER-MAJOR) fs: deprecate closing FileHandle on garbage collection
  (James M Snell) [#28396](#28396)
* (SEMVER-MAJOR) http: move OutboundMessage.prototype.flush to EOL
  (James M Snell) [#31164](#31164)
* (SEMVER-MAJOR) lib: move GLOBAL and root aliases to EOL
  (James M Snell) [#31167](#31167)
* (SEMVER-MAJOR) os: move tmpDir() to EOL
  (James M Snell) [#31169](#31169)
* (SEMVER-MAJOR) src: remove deprecated wasm type check
  (Clemens Backes) [#32116](#32116)
* (SEMVER-MAJOR) stream: move \_writableState.buffer to EOL
  (James M Snell) [#31165](#31165)
* (SEMVER-MINOR) doc: deprecate process.mainModule
  (Antoine du HAMEL) [#32232](#32232)
* (SEMVER-MINOR) doc: deprecate process.umask() with no arguments
  (Colin Ihrig) [#32499](#32499)

* (SEMVER-MAJOR) build: update macos deployment target to 10.13 for 14.x
  (AshCripps) [#32454](#32454)
* (SEMVER-MAJOR) win: block running on EOL Windows versions (João Reis)
  [#31954](#31954)

* (SEMVER-MAJOR) deps: update V8 to 8.1.307.20 (Matheus Marchini)
  [#32116](#32116)

PR-URL: #32181
BethGriggs added a commit that referenced this pull request Apr 20, 2020
- (SEMVER-MAJOR) crypto: move pbkdf2 without digest to EOL
  (James M Snell) [#31166](#31166)
- (SEMVER-MAJOR) fs: deprecate closing FileHandle on garbage collection
  (James M Snell) [#28396](#28396)
- (SEMVER-MAJOR) http: move OutboundMessage.prototype.flush to EOL
  (James M Snell) [#31164](#31164)
- (SEMVER-MAJOR) lib: move GLOBAL and root aliases to EOL
  (James M Snell) [#31167](#31167)
- (SEMVER-MAJOR) os: move tmpDir() to EOL
  (James M Snell)[#31169](#31169)
- (SEMVER-MAJOR) src: remove deprecated wasm type check
  (Clemens Backes) [#32116](#32116)
- (SEMVER-MAJOR) stream: move \_writableState.buffer to EOL
  (James M Snell) [#31165](#31165)
- (SEMVER-MINOR) doc: deprecate process.mainModule
  (Antoine du HAMEL) [#32232](#32232)
- (SEMVER-MINOR) doc: deprecate process.umask() with no arguments
  (Colin Ihrig) [#32499](#32499)

- src: migrate to new V8 ArrayBuffer API (Thang Tran)
  [#30782](#30782)

It is possible that this change will impact some native addons using
`ArrayBuffer`.

- (SEMVER-MAJOR) build: update macos deployment target to 10.13 for 14.x
  (AshCripps)[#32454](#32454)
- (SEMVER-MAJOR) doc: update cross compiler machine for Linux armv7
  (Richard Lau) [#32812](#32812)
- (SEMVER-MAJOR) doc: update Centos/RHEL releases use devtoolset-8
  (Richard Lau) [#32812](#32812)
- (SEMVER-MAJOR) doc: remove SmartOS from official binaries
  (Richard Lau) [#32812](#32812)
- (SEMVER-MAJOR) win: block running on EOL Windows versions
  (João Reis) [#31954](#31954)

It is expected that there will be an ABI mismatch on ARM between the
Node.js binary and native addons.
- [#30786](#30786)

- (SEMVER-MAJOR) deps: update V8 to 8.1.307.20
  (Matheus Marchini) [#32116](#32116)

- cli, report: move --report-on-fatalerror to stable
  (Colin Ihrig) [#32496](#32496)
- deps: upgrade to libuv 1.37.0
  (Colin Ihrig) [#32866](#32866)
- fs: add fs/promises alias module
  (Gus Caplan) [#31553](#31553)
- module: remove experimental modules warning
  (Guy Bedford) [#31974](#31974)

PR-URL: #32181
BethGriggs added a commit that referenced this pull request Apr 20, 2020
Deprecations:

- (SEMVER-MAJOR) crypto: move pbkdf2 without digest to EOL
  (James M Snell) [#31166](#31166)
- (SEMVER-MAJOR) fs: deprecate closing FileHandle on garbage collection
  (James M Snell) [#28396](#28396)
- (SEMVER-MAJOR) http: move OutboundMessage.prototype.flush to EOL
  (James M Snell) [#31164](#31164)
- (SEMVER-MAJOR) lib: move GLOBAL and root aliases to EOL
  (James M Snell) [#31167](#31167)
- (SEMVER-MAJOR) os: move tmpDir() to EOL
  (James M Snell)[#31169](#31169)
- (SEMVER-MAJOR) src: remove deprecated wasm type check
  (Clemens Backes) [#32116](#32116)
- (SEMVER-MAJOR) stream: move \_writableState.buffer to EOL
  (James M Snell) [#31165](#31165)
- (SEMVER-MINOR) doc: deprecate process.mainModule
  (Antoine du HAMEL) [#32232](#32232)
- (SEMVER-MINOR) doc: deprecate process.umask() with no arguments
  (Colin Ihrig) [#32499](#32499)

ECMAScript Modules - Experimental Warning Removal:

- module: remove experimental modules warning
  (Guy Bedford) [#31974](#31974)

In Node.js 13 we removed the need to include the --experimental-modules
flag, but when running EcmaScript Modules in Node.js, this would still
result in a warning ExperimentalWarning: The ESM module loader is
experimental.

As of Node.js 14 there is no longer this warning when using ESM in
Node.js. However, the ESM implementation in Node.js remains
experimental. As per our stability index: “The feature is not subject
to Semantic Versioning rules. Non-backward compatible changes or
removal may occur in any future release.” Users should be cautious when
using the feature in production environments.

Please keep in mind that the implementation of ESM in Node.js differs
from the developer experience you might be familiar with. Most
transpilation workflows support features such as optional file
extensions or JSON modules that the Node.js ESM implementation does not
support. It is highly likely that modules from transpiled environments
will require a certain degree of refactoring to work in Node.js. It is
worth mentioning that many of our design decisions were made with two
primary goals. Spec compliance and Web Compatibility. It is our belief
that the current implementation offers a future proof model to
authoring ESM modules that paves the path to Universal JavaScript.
Please read more in our documentation.

The ESM implementation in Node.js is still experimental but we do believe
that we are getting very close to being able to call ESM in Node.js
“stable”. Removing the warning is a huge step in that direction.

Migrate to new V8 ArrayBuffer API:

- src: migrate to new V8 ArrayBuffer API (Thang Tran)
  [#30782](#30782)

It is possible that this change will impact some native addons using
`ArrayBuffer`.

Toolchain and Compiler Upgrades:

- (SEMVER-MAJOR) build: update macos deployment target to 10.13 for 14.x
  (AshCripps)[#32454](#32454)
- (SEMVER-MAJOR) doc: update cross compiler machine for Linux armv7
  (Richard Lau) [#32812](#32812)
- (SEMVER-MAJOR) doc: update Centos/RHEL releases use devtoolset-8
  (Richard Lau) [#32812](#32812)
- (SEMVER-MAJOR) doc: remove SmartOS from official binaries
  (Richard Lau) [#32812](#32812)
- (SEMVER-MAJOR) win: block running on EOL Windows versions
  (João Reis) [#31954](#31954)

It is expected that there will be an ABI mismatch on ARM between the
Node.js binary and native addons. Native addons are only broken if they
interact with `std::shared_ptr`. This is expected to be fixed in a
later version of Node.js 14.
- [#30786](#30786)

Update to V8 8.1:

- (SEMVER-MAJOR) deps: update V8 to 8.1.307.20
  (Matheus Marchini) [#32116](#32116)

Other Notable Changes:

- cli, report: move --report-on-fatalerror to stable
  (Colin Ihrig) [#32496](#32496)
- deps: upgrade to libuv 1.37.0
  (Colin Ihrig) [#32866](#32866)
- fs: add fs/promises alias module
  (Gus Caplan) [#31553](#31553)

PR-URL: #32181
BethGriggs added a commit that referenced this pull request Apr 21, 2020
Deprecations:

- (SEMVER-MAJOR) crypto: move pbkdf2 without digest to EOL
  (James M Snell) [#31166](#31166)
- (SEMVER-MAJOR) fs: deprecate closing FileHandle on garbage collection
  (James M Snell) [#28396](#28396)
- (SEMVER-MAJOR) http: move OutboundMessage.prototype.flush to EOL
  (James M Snell) [#31164](#31164)
- (SEMVER-MAJOR) lib: move GLOBAL and root aliases to EOL
  (James M Snell) [#31167](#31167)
- (SEMVER-MAJOR) os: move tmpDir() to EOL
  (James M Snell)[#31169](#31169)
- (SEMVER-MAJOR) src: remove deprecated wasm type check
  (Clemens Backes) [#32116](#32116)
- (SEMVER-MAJOR) stream: move \_writableState.buffer to EOL
  (James M Snell) [#31165](#31165)
- (SEMVER-MINOR) doc: deprecate process.mainModule
  (Antoine du HAMEL) [#32232](#32232)
- (SEMVER-MINOR) doc: deprecate process.umask() with no arguments
  (Colin Ihrig) [#32499](#32499)

ECMAScript Modules - Experimental Warning Removal:

- module: remove experimental modules warning
  (Guy Bedford) [#31974](#31974)

In Node.js 13 we removed the need to include the --experimental-modules
flag, but when running EcmaScript Modules in Node.js, this would still
result in a warning ExperimentalWarning: The ESM module loader is
experimental.

As of Node.js 14 there is no longer this warning when using ESM in
Node.js. However, the ESM implementation in Node.js remains
experimental. As per our stability index: “The feature is not subject
to Semantic Versioning rules. Non-backward compatible changes or
removal may occur in any future release.” Users should be cautious when
using the feature in production environments.

Please keep in mind that the implementation of ESM in Node.js differs
from the developer experience you might be familiar with. Most
transpilation workflows support features such as optional file
extensions or JSON modules that the Node.js ESM implementation does not
support. It is highly likely that modules from transpiled environments
will require a certain degree of refactoring to work in Node.js. It is
worth mentioning that many of our design decisions were made with two
primary goals. Spec compliance and Web Compatibility. It is our belief
that the current implementation offers a future proof model to
authoring ESM modules that paves the path to Universal JavaScript.
Please read more in our documentation.

The ESM implementation in Node.js is still experimental but we do believe
that we are getting very close to being able to call ESM in Node.js
“stable”. Removing the warning is a huge step in that direction.

Migrate to new V8 ArrayBuffer API:

- src: migrate to new V8 ArrayBuffer API (Thang Tran)
  [#30782](#30782)

It is possible that this change will impact some native addons using
`ArrayBuffer`.

Toolchain and Compiler Upgrades:

- (SEMVER-MAJOR) build: update macos deployment target to 10.13 for 14.x
  (AshCripps)[#32454](#32454)
- (SEMVER-MAJOR) doc: update cross compiler machine for Linux armv7
  (Richard Lau) [#32812](#32812)
- (SEMVER-MAJOR) doc: update Centos/RHEL releases use devtoolset-8
  (Richard Lau) [#32812](#32812)
- (SEMVER-MAJOR) doc: remove SmartOS from official binaries
  (Richard Lau) [#32812](#32812)
- (SEMVER-MAJOR) win: block running on EOL Windows versions
  (João Reis) [#31954](#31954)

It is expected that there will be an ABI mismatch on ARM between the
Node.js binary and native addons. Native addons are only broken if they
interact with `std::shared_ptr`. This is expected to be fixed in a
later version of Node.js 14.
- [#30786](#30786)

Update to V8 8.1:

- (SEMVER-MAJOR) deps: update V8 to 8.1.307.20
  (Matheus Marchini) [#32116](#32116)

Other Notable Changes:

- cli, report: move --report-on-fatalerror to stable
  (Colin Ihrig) [#32496](#32496)
- deps: upgrade to libuv 1.37.0
  (Colin Ihrig) [#32866](#32866)
- fs: add fs/promises alias module
  (Gus Caplan) [#31553](#31553)

PR-URL: #32181
BethGriggs added a commit that referenced this pull request Apr 21, 2020
Deprecations:

- (SEMVER-MAJOR) crypto: move pbkdf2 without digest to EOL
  (James M Snell) [#31166](#31166)
- (SEMVER-MAJOR) fs: deprecate closing FileHandle on garbage collection
  (James M Snell) [#28396](#28396)
- (SEMVER-MAJOR) http: move OutboundMessage.prototype.flush to EOL
  (James M Snell) [#31164](#31164)
- (SEMVER-MAJOR) lib: move GLOBAL and root aliases to EOL
  (James M Snell) [#31167](#31167)
- (SEMVER-MAJOR) os: move tmpDir() to EOL
  (James M Snell)[#31169](#31169)
- (SEMVER-MAJOR) src: remove deprecated wasm type check
  (Clemens Backes) [#32116](#32116)
- (SEMVER-MAJOR) stream: move \_writableState.buffer to EOL
  (James M Snell) [#31165](#31165)
- (SEMVER-MINOR) doc: deprecate process.mainModule
  (Antoine du HAMEL) [#32232](#32232)
- (SEMVER-MINOR) doc: deprecate process.umask() with no arguments
  (Colin Ihrig) [#32499](#32499)

ECMAScript Modules - Experimental Warning Removal:

- module: remove experimental modules warning
  (Guy Bedford) [#31974](#31974)

In Node.js 13 we removed the need to include the --experimental-modules
flag, but when running EcmaScript Modules in Node.js, this would still
result in a warning ExperimentalWarning: The ESM module loader is
experimental.

As of Node.js 14 there is no longer this warning when using ESM in
Node.js. However, the ESM implementation in Node.js remains
experimental. As per our stability index: “The feature is not subject
to Semantic Versioning rules. Non-backward compatible changes or
removal may occur in any future release.” Users should be cautious when
using the feature in production environments.

Please keep in mind that the implementation of ESM in Node.js differs
from the developer experience you might be familiar with. Most
transpilation workflows support features such as optional file
extensions or JSON modules that the Node.js ESM implementation does not
support. It is highly likely that modules from transpiled environments
will require a certain degree of refactoring to work in Node.js. It is
worth mentioning that many of our design decisions were made with two
primary goals. Spec compliance and Web Compatibility. It is our belief
that the current implementation offers a future proof model to
authoring ESM modules that paves the path to Universal JavaScript.
Please read more in our documentation.

The ESM implementation in Node.js is still experimental but we do believe
that we are getting very close to being able to call ESM in Node.js
“stable”. Removing the warning is a huge step in that direction.

New V8 ArrayBuffer API:

* **src**: migrate to new V8 ArrayBuffer API
  (Thang Tran) [#30782](#30782)

Multiple ArrayBuffers pointing to the same base address are no longer
allowed by V8. This may impact native addons.

Toolchain and Compiler Upgrades:

- (SEMVER-MAJOR) build: update macos deployment target to 10.13 for 14.x
  (AshCripps)[#32454](#32454)
- (SEMVER-MAJOR) doc: update cross compiler machine for Linux armv7
  (Richard Lau) [#32812](#32812)
- (SEMVER-MAJOR) doc: update Centos/RHEL releases use devtoolset-8
  (Richard Lau) [#32812](#32812)
- (SEMVER-MAJOR) doc: remove SmartOS from official binaries
  (Richard Lau) [#32812](#32812)
- (SEMVER-MAJOR) win: block running on EOL Windows versions
  (João Reis) [#31954](#31954)

It is expected that there will be an ABI mismatch on ARM between the
Node.js binary and native addons. Native addons are only broken if they
interact with `std::shared_ptr`. This is expected to be fixed in a
later version of Node.js 14.
- [#30786](#30786)

Update to V8 8.1:

- (SEMVER-MAJOR) deps: update V8 to 8.1.307.20
  (Matheus Marchini) [#32116](#32116)

Other Notable Changes:

- cli, report: move --report-on-fatalerror to stable
  (Colin Ihrig) [#32496](#32496)
- deps: upgrade to libuv 1.37.0
  (Colin Ihrig) [#32866](#32866)
- fs: add fs/promises alias module
  (Gus Caplan) [#31553](#31553)

PR-URL: #32181
BethGriggs added a commit that referenced this pull request Apr 21, 2020
Deprecations:

- (SEMVER-MAJOR) crypto: move pbkdf2 without digest to EOL
  (James M Snell) [#31166](#31166)
- (SEMVER-MAJOR) fs: deprecate closing FileHandle on garbage collection
  (James M Snell) [#28396](#28396)
- (SEMVER-MAJOR) http: move OutboundMessage.prototype.flush to EOL
  (James M Snell) [#31164](#31164)
- (SEMVER-MAJOR) lib: move GLOBAL and root aliases to EOL
  (James M Snell) [#31167](#31167)
- (SEMVER-MAJOR) os: move tmpDir() to EOL
  (James M Snell)[#31169](#31169)
- (SEMVER-MAJOR) src: remove deprecated wasm type check
  (Clemens Backes) [#32116](#32116)
- (SEMVER-MAJOR) stream: move \_writableState.buffer to EOL
  (James M Snell) [#31165](#31165)
- (SEMVER-MINOR) doc: deprecate process.mainModule
  (Antoine du HAMEL) [#32232](#32232)
- (SEMVER-MINOR) doc: deprecate process.umask() with no arguments
  (Colin Ihrig) [#32499](#32499)

ECMAScript Modules - Experimental Warning Removal:

- module: remove experimental modules warning
  (Guy Bedford) [#31974](#31974)

In Node.js 13 we removed the need to include the --experimental-modules
flag, but when running EcmaScript Modules in Node.js, this would still
result in a warning ExperimentalWarning: The ESM module loader is
experimental.

As of Node.js 14 there is no longer this warning when using ESM in
Node.js. However, the ESM implementation in Node.js remains
experimental. As per our stability index: “The feature is not subject
to Semantic Versioning rules. Non-backward compatible changes or
removal may occur in any future release.” Users should be cautious when
using the feature in production environments.

Please keep in mind that the implementation of ESM in Node.js differs
from the developer experience you might be familiar with. Most
transpilation workflows support features such as optional file
extensions or JSON modules that the Node.js ESM implementation does not
support. It is highly likely that modules from transpiled environments
will require a certain degree of refactoring to work in Node.js. It is
worth mentioning that many of our design decisions were made with two
primary goals. Spec compliance and Web Compatibility. It is our belief
that the current implementation offers a future proof model to
authoring ESM modules that paves the path to Universal JavaScript.
Please read more in our documentation.

The ESM implementation in Node.js is still experimental but we do believe
that we are getting very close to being able to call ESM in Node.js
“stable”. Removing the warning is a huge step in that direction.

New V8 ArrayBuffer API:

* **src**: migrate to new V8 ArrayBuffer API
  (Thang Tran) [#30782](#30782)

Multiple ArrayBuffers pointing to the same base address are no longer
allowed by V8. This may impact native addons.

Toolchain and Compiler Upgrades:

- (SEMVER-MAJOR) build: update macos deployment target to 10.13 for 14.x
  (AshCripps)[#32454](#32454)
- (SEMVER-MAJOR) doc: update cross compiler machine for Linux armv7
  (Richard Lau) [#32812](#32812)
- (SEMVER-MAJOR) doc: update Centos/RHEL releases use devtoolset-8
  (Richard Lau) [#32812](#32812)
- (SEMVER-MAJOR) doc: remove SmartOS from official binaries
  (Richard Lau) [#32812](#32812)
- (SEMVER-MAJOR) win: block running on EOL Windows versions
  (João Reis) [#31954](#31954)

It is expected that there will be an ABI mismatch on ARM between the
Node.js binary and native addons. Native addons are only broken if they
interact with `std::shared_ptr`. This is expected to be fixed in a
later version of Node.js 14.
- [#30786](#30786)

Update to V8 8.1:

- (SEMVER-MAJOR) deps: update V8 to 8.1.307.20
  (Matheus Marchini) [#32116](#32116)

Other Notable Changes:

- cli, report: move --report-on-fatalerror to stable
  (Colin Ihrig) [#32496](#32496)
- deps: upgrade to libuv 1.37.0
  (Colin Ihrig) [#32866](#32866)
- fs: add fs/promises alias module
  (Gus Caplan) [#31553](#31553)

PR-URL: #32181
@targos targos removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 25, 2020
@Uko
Copy link

Uko commented Jul 1, 2020

Pardon my curiosity, but why "Closing the FileHandle on garbage collection is a bad practice?" I tried to google that up, but maybe smart people here can give me some pointers :)

@tniessen
Copy link
Member

tniessen commented Jul 1, 2020

@Uko The garbage collector should really only free memory that is not used anymore. Relying on the GC for cleaning up resources (e.g., closing file descriptors) is generally considered bad practice, because

  • in many implementations, when and how the GC runs is not deterministic,
  • resource cleanup might be slow and might therefore cause long GC times, and
  • resource cleanup itself might produce errors, which cannot be handled by the GC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. deprecations Issues and PRs related to deprecations. fs Issues and PRs related to the fs subsystem / file system. promises Issues and PRs related to ECMAScript promises. semver-major PRs that contain breaking changes and should be released in the next major version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.