fuel / core

Fuel PHP Framework - The core of the Fuel v1 framework
http://fuelphp.com
813 stars 345 forks source link

BC break: adding closure to Presenter's view() method #1881

Closed kenjis closed 9 years ago

kenjis commented 9 years ago

We forgot the issue: https://github.com/fuel/core/commit/30643ab08bc07d9f8ae75ca8a337166aec5f117b

On Fuel 1.7.3, "Passing functions to views" does not work. http://fuelphp.com/docs/general/presenters.html#/functions

There is no documentaion: https://github.com/fuel/fuel/blob/1.7/master/CHANGELOG.md#v173

WanWizard commented 9 years ago

I know, but I have no decent solution as of yet, as there are two use-cases which are conflicting:

Case 1: as documented, pass a closure to be used as a modifier Case 2: as implemented, pass a closure to be used a value provider

The only thing I can think of at the moment is to use Reflection, and use it to check if the closure has any arguments. If not, it's a case 2 (it can never be used as a modifier), and if it has, it's a case 1 (a value provider can not require arguments).

If a closure has arguments, but all are optional, it could be both, so a decision has to be made what this means (I would say that any argument present makes it a case 1).

Using reflection might have a performance impact.

kenjis commented 9 years ago

Why do you need case 2?

WanWizard commented 9 years ago

Case 2 is needed to be able to inject dynamic data retrieval or processing. It was the case that triggered the change back in january (I think).

Either way we're stuck in a double BC situation at the moment, because some will have a need for case 1 (and now have a problem) and some for case 2 (which will get into trouble if we revert it). So we do need a solution to cover both.

There's another difference (and that is the reason for the current code) is that a modifier function needs to be passed on as-is to the view (so always with encoding disabled) while a value function needs to be converted to a value first, to the encoding settings can be honoured.

WanWizard commented 9 years ago

We've been discussing this, and our proposal is:

We will treat a closure exactly the same way as a variable. If you pass it to a view or presenter with the filter enabled, it will be evaluated prior to being encoded. If you pass it to a view or presenter with the filter disabled (for example through set_safe), the closure will be passed to the view untouched.

For existing code, we add a BC config key, that when enabled would absorb the error when evaluating a closure meant to be a modifier, and will pass the closure without attempting to encode it. This will have a performance hit because this can only be done using reflection (to get the argument count).

kenjis commented 9 years ago

I got it. Thanks.