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

Fixes #294. #321

Merged
merged 2 commits into from
Dec 6, 2022
Merged

Fixes #294. #321

merged 2 commits into from
Dec 6, 2022

Conversation

thekevinbrown
Copy link
Contributor

@thekevinbrown thekevinbrown commented Dec 5, 2022

This PR fixes #294.

It should be noted that this also amends the max batch size respects cached results test. A side effect of this change is that it slightly changes the moment when the first batch is resolved. DataLoader still does respect cached results, and the load call in that test is still only called to request 2, so I didn't think this was a problem.

Happy to do this a different / better way if anyone has any ideas, but this seemed the most straightforward to me.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Dec 5, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: thekevinbrown / name: Kevin Brown (a5c6034)

@thekevinbrown
Copy link
Contributor Author

I've now signed the CLA ^.

@saihaj
Copy link
Member

saihaj commented Dec 5, 2022

can you run yarn changeset and add a patch and explain this change. You can read more about changeset here

@thekevinbrown
Copy link
Contributor Author

No worries!

That's done, and I also created #322 to add this info to CONTRIBUTING.md for the future.

@saihaj
Copy link
Member

saihaj commented Dec 5, 2022

awesome! @thekevinbrown I recently have been running into some npm publish issue. I will try to spend sometime this week so we can get this published

@thekevinbrown
Copy link
Contributor Author

thekevinbrown commented Dec 6, 2022

No worries, I'm working off my own local copy anyway so no stress there.

If there's anything I can do to help just let me know!

@saihaj saihaj merged commit 3cd3a43 into graphql:main Dec 6, 2022
@thekevinbrown thekevinbrown deleted the fix/294 branch December 7, 2022 00:00
@thekevinbrown
Copy link
Contributor Author

Hey @saihaj, just wanted to check in and see how the publish is going. Is there anything I can do to help?

@saihaj
Copy link
Member

saihaj commented Jan 6, 2023

I am waiting for @leebyron to update some settings in GH org

@thekevinbrown
Copy link
Contributor Author

@leebyron, any progress?

@leebyron
Copy link
Contributor

I'm not sure why version deploy is broken, but I added @saihaj as an NPM publisher so hopefully you can help debug

@saihaj
Copy link
Member

saihaj commented Jan 17, 2023

yes I received NPM access thanks @leebyron will try to get a release out sometime this week @thekevinbrown

@thekevinbrown
Copy link
Contributor Author

@saihaj, sorry to pester, just want to check in and see if there's anything I can do to help?

@saihaj
Copy link
Member

saihaj commented Feb 2, 2023

@saihaj, sorry to pester, just want to check in and see if there's anything I can do to help?

let's use this issue to chat update since there are a few things that we will rollout #328 (comment)

@thekevinbrown
Copy link
Contributor Author

Awesome, thanks for your work @saihaj! Much appreciated.

thekevinbrown added a commit to exogee-technology/graphweaver that referenced this pull request Feb 23, 2023
rich-exogee pushed a commit to exogee-technology/graphweaver that referenced this pull request Mar 8, 2023
steve-keep pushed a commit to exogee-technology/graphweaver that referenced this pull request May 3, 2023
steve-keep pushed a commit to exogee-technology/graphweaver that referenced this pull request May 3, 2023
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.

[BUG] Loader incorrectly creates a new batch before reaching maxBatchSize and batch dispatch
3 participants