search

slimphp / PHP-View

A Simple PHP Renderer for Slim 3 & 4 (or any other PSR-7 project)
MIT License
264 stars 60 forks source link

Bugfix: Infinite loop in fetch #53

Closed piotr-cz closed 4 years ago

piotr-cz commented 4 years ago

Resolves https://github.com/slimphp/PHP-View/issues/49

Layout support added in v2.2.1 in this PR changed the way how fetch method works.

When there is a layout set and it or other template file calls fetch method, it creates an infinite loop:

which results in

Fatal error: Uncaught Error: Maximum function nesting level of '512' reached, aborting!

While there is another attempt to fix the issue, https://github.com/slimphp/PHP-View/pull/52 it does so by introducing new method, while this PR introduces new argument in fetch method: $useLayout which defaults to false.

l0gicgate commented 4 years ago

I'm not familiar with this repo at all @adriansuter are you? I'd like to process this PR in a timely manner. @akrabat @geggleto please review if possible.

adriansuter commented 4 years ago

@l0gicgate Unfortunately not. I was using Twig all the time.

But the solution in this PR seems to be plausible.

adriansuter commented 4 years ago

Isn't this a BC?

If someone is extending the class and overriding the method fetch.

piotr-cz commented 4 years ago

Yes, it is a BC break in a sense that if somebody would extend fetch method and add an argument.

In that case #52 would be more appropriate and I'd add note in the docblock that fetch method must not be called from within a template when layout is used.

akrabat commented 4 years ago

Will look later today

jaywilliams commented 4 years ago

Any update on this PR?

akrabat commented 4 years ago

Sorry, I didn't get to it due to reasons that are uninteresting. It's on the list for tomorrow.

On 10 Oct 2019, at 19:05, Jay Williams notifications@github.com wrote:

 Any update on this PR?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

l0gicgate commented 4 years ago

Thank you @akrabat I'm just inclined to merge it. If it breaks things, we'll hear about it and we can fix it then.

akrabat commented 4 years ago

My general feeling is that we should roll a 3.0 for this. While we at it, we can make 3.0 PHP 7.1+ too.

heannig commented 4 years ago

This bugfix solved my issue rendering partials with version 2.2. I'd be happy if you could release this fix

l0gicgate commented 4 years ago

@heannig you will need to update to version 3.0.0 in order to get this bug fix. I just pushed a release.

heannig commented 4 years ago

@l0gicgate thank's a lot 👍