Add GotenbergAssetRuntime to avoid passing Builder in context
#128
Conversation
Passing builder in context is fragile, as it won't work if user uses any of Twig layout features without passing the full context. Starting with Twig 4.0, the `include` function won't pass the full context, to encourage template isolation and composition. This PR introduce a Twig Runtime, lowering the impact of this extension on the global scope, and allowing to create a time-limited "scope", during which the `gotenberg_asset` will work as expected.
| public function getAssetUrl(string $path): string | ||
| { | ||
| if (null === $this->builder) { | ||
| throw new \LogicException('You need to extend from AbstractChromiumPdfBuilder to use "gotenberg_asset" function.'); |
There was a problem hiding this comment.
| throw new \LogicException('You need to extend from AbstractChromiumPdfBuilder to use "gotenberg_asset" function.'); | |
| throw new \LogicException('The gotenberg_asset function must be used in a Gotenberg context. You can use it via the AbstractChromiumPdfBuilder or AbstractChromiumScreenshotBuilder class.'); |
Should be easier to understand.
There was a problem hiding this comment.
I took the original one, but yeah this looks a bit weird.
To be honest i would not specify class names here at all.. It's a bit weird no ?
And add a dedicated interface/VO like "Gotenberg\Assets" (or "AssetSet" or "TemplateAssetBag" or ...) and just use $assets->add(...).
Could even use a default one if none is set.
In the end, I think this function could very much work for everyone... the AssetBag would simply not be collected after the render of the template.
WDYT ?
There was a problem hiding this comment.
To be honest i would not specify class names here at all.. It's a bit weird no ?
Indeed it is not very helpful.
And add a dedicated interface/VO like "Gotenberg\Assets" (or "AssetSet" or "TemplateAssetBag" or ...) and just use $assets->add(...).
I hope I can have some time to rework builders in that way. I started to work with a HeadersBag and a BodyBag, but an AssetBag should be added too.
There was a problem hiding this comment.
AssetBag->addAsset($asset)
Here, $asset can be:
- A logical path: mapped by assetmapper/webpack -> Package::getUrl.
- An absolute URL: directly accepted.
- A local file: either streamed or converted into a signed URL.
This approach should simplify things quite a bit.
And then a second twig function to inline some assets (base64 images for instance, work good for header/footer things)
Passing the builder in the Twig context is a bit fragile.
It won't work when template uses any of Twig layout features (includes, embed, macros, ...) without passing the full context.
Also, starting with Twig 4.0, the
includefunction won't pass the full context per default, with no "boolean" to do so automatically, encouraging template isolation and composition.This PR introduce a Twig Runtime, lowering the impact of the Extension on the global scope, and allowing to create a time-limited "scope", during which the
gotenberg_assetwill work as expected.