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

[v2] Partial reload incorrectly evaluates props not included in response (optional & lazy) #672

Open
MarcEspiard opened this issue Oct 10, 2024 · 1 comment

Comments

@MarcEspiard
Copy link

MarcEspiard commented Oct 10, 2024

Hi,

Testing v2 beta1 released today.

Just testing deferred props and noticed that my shared props were evaluated during the loading of the deferred props (partial load), even though all my shared props are in the Optional form and shouldn't be evaluated during a partial load.

"shared_list" => fn() => ...

I've tracked the issue to the resolveProperties function in Response:

public function resolveProperties(Request $request, array $props): array
{
$isPartial = $this->isPartial($request);
if (! $isPartial) {
$props = array_filter($this->props, static function ($prop) {
return ! ($prop instanceof IgnoreFirstLoad);
});
}
$props = $this->resolveArrayableProperties($props, $request);
if ($isPartial && $request->hasHeader(Header::PARTIAL_ONLY)) {
$props = $this->resolveOnly($request, $props);
}
if ($isPartial && $request->hasHeader(Header::PARTIAL_EXCEPT)) {
$props = $this->resolveExcept($request, $props);
}
$props = $this->resolveAlways($props);
$props = $this->resolvePropertyInstances($props, $request);
return $props;
}

Calling

$props = $this->resolveArrayableProperties($props, $request);
resolves all props, no matter if they are supposed to be included in the response.

Moving $props = $this->resolveArrayableProperties($props, $request); below resolveExcept solves the issue as the props have been filtered at the point.

I don't know the potential ramifications of moving this line of code since this is a brand new refactor, but if it's OK with you I'm happy to submit a PR for the fix. Just let me know

@wizzymore
Copy link

wizzymore commented Nov 12, 2024

I just came here to report the same issue.

Instead of not running the closure on partial realods, even if you use only: [key_not_included_here] or except: [key_included_here] it will correctly NOT appear in the response, but it will be evaluated and all the code inside the closure will run.

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

No branches or pull requests

2 participants