NODE-2815: alternative no generic parameter for options #2659
Conversation
This file contains hidden or 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
mbroadst
force-pushed
the
NODE-2815/no-generic-parameter-for-options
branch
from
c482c5c to
68a550e
Compare
nbbeeken
reviewed
Dec 3, 2020
| @@ -49,21 +47,22 @@ export abstract class OperationBase< | |||
| // BSON serialization options | |||
| bsonOptions?: BSONSerializeOptions; | |||
|
|
|||
| constructor(options: T = {} as T) { | |||
| this.options = Object.assign({}, options); | |||
| [kSession]: ClientSession; | |||
There was a problem hiding this comment.
I think this should be [kSession]?: ClientSession I guess TS has some issues around symbol properties that it's not enforcing this.
mbroadst
force-pushed
the
NODE-2815/no-generic-parameter-for-options
branch
2 times, most recently
from
e7db3a9 to
6ccab2b
Compare
nbbeeken
reviewed
Dec 4, 2020
This removes the generic options property of the base class for
all operations, in favor of storing those options on the concrete
operations. The work entailed a few major changes to the codebase:
- renaming OperationBase to AbstractOperation (in keeping with
similar naming conventions for cursors)
- requiring sessions to be managed at the `execute` level for
concrete operations
- finally, removing the generic type parameter for operation
definitions
NODE-2815
mbroadst
force-pushed
the
NODE-2815/no-generic-parameter-for-options
branch
from
ea7ce79 to
f7275ce
Compare
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
This is the most basic work required to remove the generic options type for the
OperationBasetype. The general rule of thumb was to add anoptionsproperty to each subclass with a concrete type specific to that operation. Other work includes:OperationBasetoAbstractOperationin keeping with the convention introduced in theAbstractCursorworkreadPreferencebeing stored on a copy of options in the base class)