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

[5.7] Remove Blade defaults #23532

Merged
merged 1 commit into from
Mar 14, 2018
Merged

[5.7] Remove Blade defaults #23532

merged 1 commit into from
Mar 14, 2018

Conversation

browner12
Copy link
Contributor

current Blade functionality allows the user to define a default value if the first value is not set.

{{ $name or 'Andrew' }}

This feature was added in an early version of Laravel to be a shorthand for

isset($name) ? $name : 'Andrew'

PHP7 has introduced the null coalesce operator (??) which performs this exact functionality.

$name ?? 'Andrew'

While the 'or' feature was good when it was needed, it could produce unexpected results when the characters 'or' appeared anywhere in the Blade expression. This was solved with extensive regex, but was still somewhat error prone.

Now that Laravel requires PHP 7.1, we can utilize the null coalesce operator and remove the 'or' feature. User upgrade process will be a find and replace, and the number of characters will be identical.

{{ $name or 'Andrew' }}
{{ $name ?? 'Andrew' }}

current Blade functionality allows the user to define a default value if the first value is not set.

`{{ $name or 'Andrew' }}`

This feature was added in an early version of Laravel to be a shorthand for

`isset($name) ? $name : 'Andrew'`

PHP7 has introduced the null coalesce operator (`??`) which performs this exact functionality.

`$name ?? 'Andrew'`

While the 'or' feature was good when it was needed, it could produce unexpected results when the characters 'or' appeared anywhere in the Blade expression.  This was solved with extensive regex, but was still somewhat error prone.

Now that Laravel requires PHP 7.1, we can utilize the null coalesce operator and remove the 'or' feature. User upgrade process will be a find and replace, and the number of characters will be identical.

```
{{ $name or 'Andrew' }}
{{ $name ?? 'Andrew' }}
```
@GrahamCampbell GrahamCampbell changed the title remove Blade defaults [5.7] remove Blade defaults Mar 13, 2018
@browner12
Copy link
Contributor Author

I went back and forth on removing some of the additional tests. They are testing to make sure that 'or' inside of a string or as a variable name are not compiled into the ternary. I'm not sure they are really testing anything anymore, and it might be confusing for someone coming into the file to see them in the future.

If people agree, I'll remove them as well.

@taylorotwell taylorotwell merged commit ec5764c into laravel:master Mar 14, 2018
@browner12
Copy link
Contributor Author

Achievement unlocked. Get a PR merged that only removes code!

@browner12 browner12 deleted the blade-or branch March 14, 2018 14:10
@SeanJA
Copy link

SeanJA commented Mar 27, 2018

But like... why break people's templates unnecessarily?

@browner12
Copy link
Contributor Author

because it causes a lot of issues, and PHP now offers a native feature that performs this more reliably.

@mavinothkumar
Copy link

Ok. But why removing? Let it be somewhere alternative way to achieve the newer functionality. Definitely this will break many templates and hard to find and convert for bigger websites.

@tomgrohl
Copy link

It will only break for people that don't follow the upgrade guide when migrating from 5.6 to 5.7.

@MadMikeyB
Copy link
Contributor

Can we not keep previous logic in addition to the new logic and mark it as deprecated in the spirit of laravel/ideas#383 ?

@GrahamCampbell GrahamCampbell changed the title [5.7] remove Blade defaults [5.7] Remove Blade defaults May 20, 2018
@antonkomarev
Copy link
Contributor

antonkomarev commented May 20, 2018

@MadMikeyB because 5.7 is a next major release and its not that hard to search for or and replace it with ??. Or even write regular expression to make it even faster.

This should significantly increase performance of big projects because it removes regular expressions and replaces.

@MadMikeyB
Copy link
Contributor

Understood @a-komarev however this will introduce BC breaks which can be avoided by deprecating first before entirely removing.

@GrahamCampbell
Copy link
Member

It has been deprecated since PHP 7.0 added support for ??.

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.

8 participants