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

upgraded to support @apollo/server 4.4.1 #41

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

geoyws
Copy link

@geoyws geoyws commented Mar 9, 2023

Do give this a try and let me know if it works! Basically we just have to change the Apollo plugin's read and write to contextValue instead of context. @slaypni @t2y Thanks so much for maintaining this gem of a library. It really saves my team a lot of time. 🙏

@geoyws
Copy link
Author

geoyws commented Apr 6, 2023

@t2y @slaypni it's a breaking change for sure, perhaps we could bump the major version number and older Apollo version users can still use the older version of type-graphql-dataloader 😉. We'll have to note it in README.md as well.

@RodoVJ
Copy link

RodoVJ commented May 9, 2023

Hi @t2y @slaypni , as @geoyws mentioned, thank you for maintaining this library. It truly is a gem! I was wondering if there are any updates on the state of this PR? My team is migrating to Apollo Server v4 and incompatibility with this library is the only thing preventing us from migrating.

Would appreciate any updates. Again, thank you for the support and this PR!

@geoyws
Copy link
Author

geoyws commented May 12, 2023

@RodoVJ perhaps you could fork and apply the changes on my pull request while we wait for the merge. My team has been actually going into the node_modules and applying these changes manually each time we pnpm install (if from scratch). 😅

@RodoVJ
Copy link

RodoVJ commented May 15, 2023

Thanks for the suggestion @geoyws 😅. We might resort to doing what your team is currently doing if we don't hear anything back soon :)

@geoyws
Copy link
Author

geoyws commented Jun 21, 2023

image
Just in case anyone wants to see where the .js code needs changing. 😬

@geoyws
Copy link
Author

geoyws commented Jul 12, 2023

My sincere apologies @slaypni , I didn't run the tests after putting in this PR.
I foolishly assumed changing the plugin was all that mattered.
I'm going to fix all the build errors and test errors first.

@lawrentiy
Copy link

My sincere apologies @slaypni , I didn't run the tests after putting in this PR. I foolishly assumed changing the plugin was all that mattered. I'm going to fix all the build errors and test errors first.

We all are waiting for your PR! ))
Thanx for u and other maintainers!

@danbopes
Copy link

Can we please get this merged in?

@ctkc
Copy link

ctkc commented Sep 13, 2023

Thank you @geoyws!

I really need this. Can we merge it? cc @slaypni @t2y

@Iran-110
Copy link
Contributor

Iran-110 commented Dec 27, 2023

@geoyws, thanks for your implementation of the new apollo v4 plugin.
I searched and find an available npm package which can be installed by yarn add @ejekanshjain/type-graphql-dataloader.

@geoyws
Copy link
Author

geoyws commented Jan 10, 2024

Finally! After half a year of marathon work on my main job I finally have the time to run tests here and make sure they're passing. Sorry again everyone.

The tests are passing now so if the changes are acceptable, please consider accepting! 🙏🏼

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.

6 participants