Leaking result cache metadata #588
Conversation
| return $callback($value, $args, $context, $info); | ||
| }); | ||
| $renderContext = new RenderContext(); | ||
| $result = $callback($value, $args, $context, $info); |
There was a problem hiding this comment.
Moving that outside of the render context kinda defeats the whole point, no?
There was a problem hiding this comment.
The result is either a callable or a generator. And the generator is evaluated further down in iterator_to_array. Thats what I discovered when writing the tests.
There was a problem hiding this comment.
So thats what has to be wrapped in the render context.
There was a problem hiding this comment.
In that case, let's move the line with new RenderContext() down there too.
There was a problem hiding this comment.
I'm okay with that 😃
Codecov Report
@@ Coverage Diff @@
## 8.x-3.x #588 +/- ##
===========================================
+ Coverage 80.4% 80.44% +0.03%
===========================================
Files 185 185
Lines 2848 2853 +5
===========================================
+ Hits 2290 2295 +5
Misses 558 558
Continue to review full report at Codecov.
|
|
@pmelab I updated the PR. I think we need to wrap the whole thing. |
| protected function resolveDeferred(callable $callback, $value, array $args, ResolveContext $context, ResolveInfo $info) { | ||
| $renderContext = new RenderContext(); | ||
| return $this->getRenderer()->executeInRenderContext($renderContext, function () use ($value, $args, $context, $info) { | ||
| $result = $callback($value, $args, $context, $info); |
There was a problem hiding this comment.
I think you need to add $callback and $renderContext to the use statement.
|
|
||
| return $this->unwrapResult($result, $info); | ||
| return $this->unwrapResult($result, $info); | ||
| } |
There was a problem hiding this comment.
build fails for syntax error ... I think you're missing ); here.
|
I shouldn't try and edit this on my windows dual boot from the github text editor :D |
No description provided.