Skip to content

fix: check for preloaded AppContext::get_loader() #3384

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.

Sign up for GitHub

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

Jump to bottom
Merged
jasonbahl merged 1 commit into from
Jun 16, 2025
Merged

fix: check for preloaded AppContext::get_loader() #3384

jasonbahl merged 1 commit into from
Jun 16, 2025
+24 −23

Conversation

@justlevine
Copy link
Collaborator

@justlevine justlevine commented Jun 6, 2025

What does this implement/fix? Explain your changes.

This PR fixes a regression in #3380 where custom dataloaders registered via the legacy hook were not getting recognized.

Does this close any currently open issues?

Follow up to #3380

Any other comments?

@justlevine justlevine requested a review from Copilot June 6, 2025 22:34
Copilot AI reviewed Jun 6, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a regression where custom preloaded dataloaders registered via legacy hooks were not recognized.

  • Updated the existence check in AppContext::get_loader() to consider both loader classes and preloaded loaders.

@justlevine justlevine added status: in review Awaiting review before merging or closing needs: reviewer response This needs the attention of a codeowner or maintainer scope: api Issues related to access functions, actions, and filters regression Bug that causes a regression to a previously working feature labels Jun 6, 2025
@qlty-cloud-legacy
Copy link

qlty-cloud-legacy bot commented Jun 6, 2025

Code Climate has analyzed commit 5b0f491 and detected 0 issues on this pull request.

View more on Code Climate.

@justlevine justlevine requested a review from jasonbahl June 6, 2025 22:43
@coveralls
Copy link

coveralls commented Jun 6, 2025

Coverage Status

coverage: 84.14%. remained the same
when pulling 5b0f491 on AxeWP:fix/AppContent--get_loader()
into a256f44 on wp-graphql:develop.

@jasonbahl jasonbahl merged commit b769d8a into wp-graphql:develop Jun 16, 2025
38 checks passed
@jasonbahl jasonbahl mentioned this pull request Jun 16, 2025
Merged
@justlevine justlevine deleted the fix/AppContent--get_loader() branch June 16, 2025 17:57
pull bot pushed a commit to Zezo-Ai/wp-graphql that referenced this pull request Jun 16, 2025
@actions-user
chore: update changeset for PR wp-graphql#3384
ccdffe1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@jasonbahl jasonbahl jasonbahl approved these changes

Assignees

No one assigned

Labels

needs: reviewer response This needs the attention of a codeowner or maintainer regression Bug that causes a regression to a previously working feature scope: api Issues related to access functions, actions, and filters status: in review Awaiting review before merging or closing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@justlevine @coveralls @jasonbahl