-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
feat(NODE-5614): Add support for explicit resource management #4177
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
baileympearson
force-pushed
the
beta-in-package-poc
branch
from
July 16, 2024 17:38
b29655f
to
f113a2b
Compare
baileympearson
changed the title
explicit resource management POC
feat(NODE-5614): Add support for explicit resource management
Jul 24, 2024
baileympearson
force-pushed
the
beta-in-package-poc
branch
from
July 24, 2024 21:53
0984c22
to
0c53c1a
Compare
nbbeeken
added
the
Primary Review
In Review with primary reviewer, not yet ready for team's eyes
label
Jul 25, 2024
nbbeeken
requested changes
Jul 25, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty much good to go! two small functional changes and some api-extractor fixes.
nbbeeken
requested changes
Jul 25, 2024
nbbeeken
previously approved these changes
Jul 26, 2024
nbbeeken
added
Team Review
Needs review from team
and removed
Primary Review
In Review with primary reviewer, not yet ready for team's eyes
labels
Jul 26, 2024
aditi-khare-mongoDB
requested changes
Jul 29, 2024
dariakp
requested changes
Jul 29, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some meta comments ahead of code review proper
dariakp
requested changes
Jul 29, 2024
baileympearson
force-pushed
the
beta-in-package-poc
branch
from
July 30, 2024 17:56
2f075ac
to
8e20e0c
Compare
This was referenced Oct 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
What is changing?
This PR adds support for explicit resource management to our async disposable resources as outlined in the design.
Beta Namespace
A new entrypoint for the driver has been added,
mongodb/lib/beta
. This namespace will contain any experimental features our APIs we want to expose to users.API extractor natively supports tagging things as
beta
and producing a beta namespace, so this PR configures API extractor to emit a beta namespace for us.Users can access the beta namespace by importing from
mongodb/lib/beta
.Symbol.asyncDispose methods
Symbol.asyncDispose methods have been added to the MongoClient, ClientSession, Cursors and ChangeStream classes. These methods are conditionally defined only when
Symbol
has anasyncDispose
property (i.e., the user has polyfilled this property themselves or they're running on a compatible version of Node.js where it's available).Our readable streams (streaming cursors and download streams) are also disposable on compatible versions of Node.js because they inherit
Symbol.asyncDispose
fromNode.js.Readable
.Additionally, a method is provided to users to allow them to attach the
Symbol.asyncDispose
methods to each class manually, if desired. This avoids complications for users when the user is polyfillingSymbol.asyncDispose
but they may import the driver (and load its classes) before the polyfill runs. In this case, since Symbol.asyncDispose is not defined when evaluating the driver's source code, no disposal methods are added. TheconfigureExplicitResourceManagement
can be called manually to add the disposal methods to each class in this scenario.Testing
There are three major CI and testing changes:
await using
and that they are cleaned up appropriately.Release Highlight
Support for explicit resource management
The driver now natively supports explicit resource management for
MongoClient
,ClientSession
,ChangeStreams
and cursors. Additionally, on compatible Node.js versions, explicit resource management can be used withcursor.stream()
and theGridFSDownloadStream
, since these classes inherit resource management from Node.js' readable streams.This feature is experimental and subject to changes at any time. This feature will remain experimental until the proposal has reached stage 4 and Node.js declares its implementation of async disposable resources as stable.
To use explicit resource management with the Node driver, you must:
tslib
polyfills for your applicationSymbol.asyncDispose
(see the TS 5.2 release announcement for more information).Explicit resource management is a feature that ensures that resources' disposal methods are always called when the resources' scope is exited. For driver resources, explicit resource management guarantees that the resources' corresponding
close
method is called when the resource goes out of scope.The full explicit resource management proposal can be found here.
Double check the following
npm run check:lint
scripttype(NODE-xxxx)[!]: description
feat(NODE-1234)!: rewriting everything in coffeescript