-
Notifications
You must be signed in to change notification settings - Fork 9
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
ISSUE-207-1.3 --- Moved twig context alter hook inside the render con… #393
Conversation
…text to avoid potential cache 'leakage' errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small change. Not a biggie but just bc in the future executeInRenderContext might change signature and not return an empty array. Thanks
// In case someone decided to wipe the original context? | ||
// We bring it back! | ||
$context = $context + $original_context; | ||
$cacheabledata = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@patdunlavey can you keep please $cacheabledata = []; in line 281?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm afraid I don't understand. Restoring it to a position before $this->renderer->executeInRenderContext
will simply result in it being overwritten, and putting it after will clobber the result of $this->renderer->executeInRenderContext
, and putting it inside the inline function does nothing that I can see.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind. I've gone ahead and updated the issue branch with the change as requested (and as I understand it).
See #392 for 1.2.0 PR