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

Apollo Server 4 Upgrade #123

Merged
merged 23 commits into from
Oct 4, 2023
Merged

Apollo Server 4 Upgrade #123

merged 23 commits into from
Oct 4, 2023

Conversation

nnoce14
Copy link
Contributor

@nnoce14 nnoce14 commented May 9, 2023

Made changes to MongoDataSource to be compliant with how data sources were changed in Apollo Server 4. See latest comment on Issue #114 for more information.

index.d.ts Outdated
@@ -1,21 +1,21 @@
declare module 'apollo-datasource-mongodb' {
import { DataSource } from 'apollo-datasource'
declare module 'apollo-mongo-datasource' {

Choose a reason for hiding this comment

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

Isnt this supposed to be apollo-datasource-mongodb itself since we are making changes to the same package?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, I was originally going to create a separate package because I wasn't sure this repo was still being maintained. Just fixed the module name, should be good now

Choose a reason for hiding this comment

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

IMO, I think it would be better to maintain this as a separate package, since I dont see this getting merged anytime soon.
This is something my team has also been dependent on.

@lorensr
Copy link
Member

lorensr commented Jul 6, 2023

@nnoce14 thanks so much for submitting! and @devileye98 for reviewing 🙏

Do we need any more automated tests to make sure this works with AS v4?

Could you also add a note to the readme saying which versions of this package support which AS versions, and a link to the old (current) version of the README/docs?

@devileye98
Copy link

ed tests to make sure this w

I forked out this PR and tested it against our application. Apart from a few minor changes in our types, we were able to get the server up and running and were also able to run our E2E and unit tests

Comment on lines +136 to +148
test('Data Source with Context', async () => {
const users = new Users({ modelOrCollection: UserModel, context: { token: '123' }})

expect(users.context.token).toBe('123')
})

test('Data Source with Context that contains a User', async () => {
const users = new Users({ modelOrCollection: userCollection, context: { user: alice }})
const user = await users.findOneById(alice._id)

expect(user.name).toBe('Alice')
expect(users.context.user.name).toBe(user.name)
})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lorensr I added these two tests to check the behavior for providing Context to datasources in Apollo Server 4. I also made minor changes to the construction of the Users datasource. Let me know if you think more tests are necessary. I also added the old README file and added a note to both which explains the versioning.

Copy link

@devileye98 devileye98 left a comment

Choose a reason for hiding this comment

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

I see that both mongodb and mongoose are on older versions. If we could update it to the newest packages, it would be awesome as we can make use of the newer types from both these packages and get rid of any type errors for users consuming this package.

@nnoce14
Copy link
Contributor Author

nnoce14 commented Jul 7, 2023

I see that both mongodb and mongoose are on older versions. If we could update it to the newest packages, it would be awesome as we can make use of the newer types from both these packages and get rid of any type errors for users consuming this package.

I just updated mongoose from v5 to v7, which is latest. I also updated mongodb from v3 to v4. However, the latest version is v5 but I was unable to get that to work because of an ongoing issue with BSON v5 found here

@devileye98
Copy link

I see that both mongodb and mongoose are on older versions. If we could update it to the newest packages, it would be awesome as we can make use of the newer types from both these packages and get rid of any type errors for users consuming this package.

I just updated mongoose from v5 to v7, which is latest. I also updated mongodb from v3 to v4. However, the latest version is v5 but I was unable to get that to work because of an ongoing issue with BSON v5 found here

I think we resolved this by updating the Bson package. I think that should resolve the issue.

@nnoce14
Copy link
Contributor Author

nnoce14 commented Jul 10, 2023

I see that both mongodb and mongoose are on older versions. If we could update it to the newest packages, it would be awesome as we can make use of the newer types from both these packages and get rid of any type errors for users consuming this package.

I just updated mongoose from v5 to v7, which is latest. I also updated mongodb from v3 to v4. However, the latest version is v5 but I was unable to get that to work because of an ongoing issue with BSON v5 found here

I think we resolved this by updating the Bson package. I think that should resolve the issue.

Yeah that seemed to work, thank you. I updated bson as well the other day, but it was still throwing me the same error at the time. All good now, mongodb is latest version.

@devileye98
Copy link

@lorensr , can we get this PR merged since I think all the comments are addressed?

@NickDuBois
Copy link

Any hope of this getting merged?

lorensr added a commit that referenced this pull request Oct 4, 2023
@lorensr lorensr merged commit 235244e into GraphQLGuide:master Oct 4, 2023
1 check passed
@lorensr
Copy link
Member

lorensr commented Oct 4, 2023

Sorry folks, very low on time these days! Lmk if anyone's interested in helping out with maintenance. Thanks for the ping Nick.

0.6.0 is now released with this PR. Please comment with how it works for you!

https://github.com/GraphQLGuide/apollo-datasource-mongodb/releases/tag/0.6.0

https://www.npmjs.com/package/apollo-datasource-mongodb?activeTab=versions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants