[HttpKernel] render_esi should behave in dev as it does in prod #15056
Conversation
|
+1 for this feature @bpaulin |
|
Adding an option only makes things more difficult for no real benefit. Either we think that the current behavior is best, and no need to do anything, or we think the new behavior is much better and it should be the default. |
|
I agree and I think the new behaviour would be better. I can rebase this pr and set this as default if needed Thanks! |
|
Hello again, @fabpot : I'm not sure if this pr should be on 2.7 now, there is a BC break after all. |
57cac09 to
971e1d6
Compare
| @@ -59,6 +59,11 @@ public function render($uri, Request $request, array $options = array()) | |||
| // want that as we want to preserve objects (so we manually set Request attributes | |||
| // below instead) | |||
| $attributes = $reference->attributes; | |||
| if (isset($options['is_fallback']) && $options['is_fallback'] ) | |||
| { | |||
There was a problem hiding this comment.
Not sure why fabbot did not catch it, but we put opening braces on the same line as the if
I suggest we treat it as a bug and consistently throw and exception in all environments (or at least, fallback to inline renderer consistently). We don't need an option here. There's also no BC break since no one could possibly rely on this inconsistency. If a bug like this slipped through dev, it would be discovered on production, investigated and fixed. |
|
I agree. This should be consistent regardless the environment. |
|
closing in favor of #16283 |
…es to the ESI and SSI renderers (jakzal) This PR was merged into the 3.1-dev branch. Discussion ---------- [HttpKernel] Deprecate passing objects as URI attributes to the ESI and SSI renderers | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #15056 | License | MIT | Doc PR | - Commits ------- a38d96e [HttpKernel] Deprecate passing objects as URI attributes to the ESI and SSI renderers
Hello,
If an object can't be turned into a string for the URL, then calling render_esi would throw an exception.
But in dev env, render_esi silently falls back on using a InlineFragmentRenderer and everything seems to work.
I add a parameter (framework.fragments.strict) to make the fallback throw the same exception, to detect the logic error in dev mode, before deploying on an env with cache enabled.