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(NODE-3843): make GridFSBucketWriteStream.prototype.end() return this for compat with community nodejs types #3088

Merged
merged 1 commit into from
Jan 4, 2022
Merged

fix(NODE-3843): make GridFSBucketWriteStream.prototype.end() return this for compat with community nodejs types #3088

merged 1 commit into from
Jan 4, 2022

Conversation

vkarpov15
Copy link
Contributor

Description

Currently GridFSBucketWriteStream.prototype.end() returns void. https://github.com/DefinitelyTyped/DefinitelyTyped/pull/57473/files changed it so that end() now must return this, which means Mongoose and the Node driver are now incompatible with @types/[email protected]. First noticed this here

What is changing?

See above

Is there new documentation needed for these changes?

No

What is the motivation for this change?

Compilation fails with @types/[email protected]. I took a look and it looks like this change compiles fine with @types/[email protected] as well.

Double check the following

  • Ran npm run check:lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: <type>(NODE-xxxx)<!>: <description>
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@baileympearson baileympearson added the External Submission PR submitted from outside the team label Jan 3, 2022
@marek-benes
Copy link

Please do review it and publish a new version. Affected projects are broken.

Copy link
Contributor

@baileympearson baileympearson left a comment

Choose a reason for hiding this comment

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

Thanks for submitting this change!

This looks good to me. I'll bring it to the team's attention for a second review and hopefully we can get a solution out in a timely manner.

@nbbeeken nbbeeken changed the title fix(gridfs): make GridFSBucketWriteStream.prototype.end() return this for compat with @types/[email protected] fix(NODE-3843): make GridFSBucketWriteStream.prototype.end() return this for compat with community nodejs types Jan 4, 2022
@baileympearson
Copy link
Contributor

Our plan is to release these changes in our next minor release, which we're targeting for Thursday. I'm going to go ahead and merge this into main but we can continue the conversation here as needed.

In the meantime, would pinning the version of @node/types to an older version work as a temporary solution?
@vkarpov15 @marek-benes

@baileympearson baileympearson merged commit 7bb9e37 into mongodb:main Jan 4, 2022
@vkarpov15
Copy link
Contributor Author

@baileympearson we pinned @types/node to < 17.0.6 in Mongoose, so I think it should work if this project pins in the meantime as well

@Fazatholomew
Copy link

@baileympearson I can confirm that on my NestJS project which uses Mongoose 6.1.4, only <16.11.7 works. @types/node 17x has the problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
External Submission PR submitted from outside the team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants