镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

fix: keep prefetch results per argument set - #845

Merged
oojacoboo merged 1 commit into
thecodingmachine:masterfrom
oojacoboo:fix/prefetch-result-per-arguments
Nov 3, 2026
Merged

oojacoboo merged 1 commit into
thecodingmachine:masterfrom
oojacoboo:fix/prefetch-result-per-arguments

Conversation

@oojacoboo

Copy link
Copy Markdown
Collaborator

Problem

When one object selects a #[Prefetch] field twice with different arguments, both aliases get the first alias's result:

contacts {
    foo: repeatName(prefix: "foo", suffix: "")
    bar: repeatName(prefix: "bar", suffix: "")
}

bar returns fooJoe instead of barJoe.

PrefetchBuffer groups the sources by a hash of the arguments (and the operation), so each alias does run its own prefetch method. But it stores the prefetch result in a WeakMap keyed by the source alone. When the second alias's Deferred runs, hasResult($source) is already true from the first alias, so PrefetchDataParameter returns that result without calling the prefetch method for the second argument set.

Fix

PrefetchBuffer stores results per source and per argument hash, using the same computeHash() that groups the sources. storeResult(), hasResult(), getResult() and purgeResult() take $arguments and ResolveInfo|null $info as optional trailing parameters, and PrefetchDataParameter passes the field's arguments and ResolveInfo. hasResult() uses array_key_exists(), so a null prefetch result still counts as stored.

Aliases with the same arguments still share one prefetch call and one result.

Compatibility

Callers of the result methods keep working: the new parameters are optional, and calls without them read and write the entry for an empty argument set. A subclass that overrides one of the four result methods would need the new parameters added to its signature.

Tests

  • EndToEndTest::testPrefetchingSameFieldWithDifferentArguments: two aliases of repeatName with different prefixes, plus a third alias sharing the first one's prefix. Fails on master (bar returns fooJoe).
  • PrefetchBufferTest::testResultsPerArguments: results stored, read and purged per argument set, including a null result.
  • PrefetchDataParameterTest::testResolveWithExistingResult stores its result with the arguments it resolves with.

Run locally on PHP 8.5 with composer update and composer update --prefer-lowest: phpunit 731 OK, phpstan no errors, phpcs clean.

PrefetchBuffer grouped sources by argument hash, so each alias of a #[Prefetch] field ran its
own prefetch, but stored the result by source alone. When one object selected the field twice
with different arguments (`a: field(x: 1) b: field(x: 2)`), the second alias found the first
alias's result and returned it.

Results are now stored per source and argument hash, the same hash that groups the sources.
storeResult(), hasResult(), getResult() and purgeResult() take the arguments and ResolveInfo as
optional trailing parameters.
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.13%. Comparing base (53f9d49) to head (b5f1261).
⚠️ Report is 184 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #845      +/-   ##
============================================
- Coverage     95.72%   92.13%   -3.60%     
- Complexity     1773     2165     +392     
============================================
  Files           154      204      +50     
  Lines          4586     5757    +1171     
============================================
+ Hits           4390     5304     +914     
- Misses          196      453     +257     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@oojacoboo
oojacoboo merged commit efe42ab into thecodingmachine:master Nov 3, 2026
12 checks passed
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.

2 participants