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

Fix: Type of Connection.transaction() #11825

Conversation

dwrss
Copy link
Contributor

@dwrss dwrss commented May 23, 2022

The documentation and code say it accepts "options" as a second parameter, but the type definition doesn't allow it.

Summary

The documentation lists Connection.prototype.transaction() as accepting an options parameter (and this is indeed the case), but the type definition doesn't allow it.
This is particularly problematic for using this function with causally consistent sessions, given that the default read concern is local, because it prevents setting the readConcern at anything other than the client level.

Examples
The sample from the documentation:

await db.transaction(async function setRank(session) {
  doc.rank = 'Captain';
  await doc.save({ session });
  doc.isNew; // false

  // Throw an error to abort the transaction
  throw new Error('Oops!');
},{ readPreference: 'primary' }).catch(() => {});

will fail to transpile as Typescript, with the error TS2554: Expected 1 arguments, but got 2.

The documentation and code say it accepts "options" as a second parameter, but the type definition doesn't allow it.
@AbdelrahmanHafez AbdelrahmanHafez added the typescript Types or Types-test related issue / Pull Request label May 23, 2022
@AbdelrahmanHafez AbdelrahmanHafez added this to the 6.3.5 milestone May 23, 2022
Copy link
Collaborator

@AbdelrahmanHafez AbdelrahmanHafez left a comment

Choose a reason for hiding this comment

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

Welcome, @dwrss
LGTM, thanks! 👍

@AbdelrahmanHafez AbdelrahmanHafez merged commit f1ece50 into Automattic:master May 23, 2022
@dwrss dwrss deleted the fix-connection-transaction-type-definition branch May 23, 2022 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
typescript Types or Types-test related issue / Pull Request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants