Skip to content
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

[11.x] add lazy default to when helper #52747

Merged
merged 2 commits into from
Sep 12, 2024
Merged

Conversation

rodrigopedra
Copy link
Contributor

@rodrigopedra rodrigopedra commented Sep 11, 2024

PR #52665 added a new helper to Laravel: when()

It simplifies the evaluation of a condition to output a value similar to PHP's ternary operator, with the false branch always returning null.

This PR:

  • Augments the helper feature set by allowing a developer to define a default value in case the conditions evaluates to false.
  • Aligns the implementation with the Illuminate\Support\Traits\Conditionable@when() method, by passing the $condition value to the value() helper, so it is passed down as an argument to any \Closure used as the true or default values.
  • Adds new test assertions to account for the newly added parameter

Notes

Why not use the null-coalesce (??) operator?

One could argue that the $default parameter is not needed, as a developer could use the null-coalesce operator instead, like this:

{{ when($user, fn() => $user->preference('theme')) ?? 'light' }}

But that doesn't allow for the lazy evaluation of the $default value, in case we want to use a dynamic default value, for example, when using Pennant to test a new design:

{{ when(Auth::user(), fn($user) => $user->preference('theme'), fn() => Feature::active('site-redesign') ? 'new-theme' : 'old-theme') }}

In the example above, the $default value would be lazily evaluated when a guest is visiting the application.

Also, as we are passing the $condition value down to the truthy \Closure, we don't need to Auth::user again.

Breaking change

As a new parameter is added, it could be considered a breaking change.

But the when() helper was just added in today's release, also the $default parameter is optional with a null default value, preserving the original behavior.

Disclaimer

I suggested this implementation in the original PR in this comment: #52665 (comment)

But, although the author of that PR was very welcoming of all suggestions, the addition of the $default value didn't make the cut.

Post Script

I changed some of the original test assertions to use assertNull(...) instead of assertEquals(null, ...).

@devajmeireles
Copy link
Contributor

I would use this PR to improve the description, I don't think the term "Output" makes sense in the description.

Also, I think $condition could have been named $boolean, like some other methods that deal with this kind of logic.

@taylorotwell taylorotwell merged commit 982710a into laravel:11.x Sep 12, 2024
31 checks passed
Comment on lines -251 to +255
return value($output);
return value($value, $condition);
}

return null;
return value($default, $condition);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just

return value($condition ? $value : $default, $condition);

?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants