Skip to content

Use value() helper in 'when' method #55465

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

Merged

Conversation

mohammadrasoulasghari
Copy link
Contributor

@mohammadrasoulasghari mohammadrasoulasghari commented Apr 18, 2025

When using the when method, it first checked if the given value was a Closure and then executed it.
This is essentially the same logic as value() helper, which was being re-implemented here.

function value($value, ...$args)
{
return $value instanceof Closure ? $value(...$args) : $value;
}

This PR replaces that duplicated logic with a direct call to value().

similar in #54650

@taylorotwell taylorotwell merged commit 48347e5 into laravel:12.x Apr 18, 2025
60 checks passed
@rodrigopedra
Copy link
Contributor

@mohammadrasoulasghari the difference is that one can use the illuminate/conditionable component in a project outside of Laravel without requiring the full illuminate/support component, or the illuminate/collections component, where the helper() value is defined.

In that case, any project requiring the illuminate/conditionable component, but none of the other components mentioned above, will break, as the value() helper won't be available.

cc @taylorotwell

@rodrigopedra
Copy link
Contributor

List of packages that are dependent on illuminate/conditionable:

https://packagist.org/packages/illuminate/conditionable/dependents?order_by=downloads

@mohammadrasoulasghari
Copy link
Contributor Author

@rodrigopedra Thank you for carefully reviewing the changes and accuracy.

As shown in the link you provided, illuminate/collections is already listed as a dependency, correct?
image

And the value() helper is defined within that package at this path:
https://github.com/illuminate/collections/blob/33af474390fabf0b17b963f84e78952489147d86/helpers.php#L234

So just to clarify — if illuminate/collections is indeed a required dependency of the package, and value() is part of that package, where exactly is the issue?

@rodrigopedra
Copy link
Contributor

@mohammadrasoulasghari, it is actually the opposite.

The link lists the packages which are dependent upon illuminate/conditionable. That means illuminate/collections depends on illuminate/conditionable, not the opposite.

You can check illuminate/conditionable doesn't depend on illuminate/collections on its composer.json file:

"require": {
"php": "^8.2"
},

@rodrigopedra
Copy link
Contributor

Also, the link shows only published packages which depend on illuminate/conditionable.

There also can be other projects, not published as packages, which depend on illuminate/conditionable.

@mohammadrasoulasghari
Copy link
Contributor Author

Also, the link shows only published packages which depend on illuminate/conditionable.

There also can be other projects, not published as packages, which depend on illuminate/conditionable.

Ah yes, you're right — I was wrong about this.
I'll create a PR to revert these changes.

mohammadrasoulasghari added a commit to mohammadrasoulasghari/laravel-framework that referenced this pull request Apr 22, 2025
@mohammadrasoulasghari mohammadrasoulasghari deleted the when-method-using-value branch April 22, 2025 15:44
taylorotwell pushed a commit that referenced this pull request Apr 23, 2025
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.

3 participants