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

[8.x] Fixes for Stringable (updated) #37613

Merged
merged 5 commits into from
Jun 7, 2021
Merged

[8.x] Fixes for Stringable (updated) #37613

merged 5 commits into from
Jun 7, 2021

Conversation

lukeraymonddowning
Copy link
Contributor

@lukeraymonddowning lukeraymonddowning commented Jun 6, 2021

This is an extension of (and replacement for) #37603.

One of the concerns raised was the amount of times the BladeCompiler would need to be resolved from the container. In this PR, if an echo handler has been defined, the compiled view will have a variable prepended to it with the value of the blade compiler, and all handler statements will then reference this variable rather than resolving a fresh instance.

I have kept the reformatting from @ibrasho's PR.

This also handles the semicolon issue mentioned in livewire/livewire#2935 by removing the semicolon at the end of an echo statement.

Here is an example of the new compiled output when an echo handler is found.

<?php $__bladeCompiler = app('blade.compiler'); ?><!DOCTYPE html>
<html lang="<?php echo e($__bladeCompiler->applyEchoHandler(str_replace('_', '-', app()->getLocale()))); ?>">
// etc...

@lukeraymonddowning
Copy link
Contributor Author

I've also taken the opportunity to add further test cases for echo handling.

@lukeraymonddowning
Copy link
Contributor Author

If having the declaration at the top is undesirable, we could also update the stringable method declaration with something like:

public function stringable($class, $handler = null)
{
    if ($class instanceof Closure) {
        [$class, $handler] = [$this->firstClosureParameterType($class), $class];
    }

    $this->echoHandlers[$class] = $handler;

    app('view')->share('__bladeCompiler', app('blade.compiler'));
}

which would achieve exactly the same result (caching the bladeCompiler) but move its declaration outside of the compiled template.

@lukeraymonddowning
Copy link
Contributor Author

@ibrasho has updated the previous PR with a different method of accomplishing this, so if you prefer that way feel free to close this PR 👍

@mewejo
Copy link

mewejo commented Jun 7, 2021

Does this also fix ternary conditionals in Blade? I had a few cases of the following failing since this update (as I was so excited to to use this great feature, thank you!):

{{ $model->exists ? 'Update' : 'Create' }}

Resulting in this situation: https://stackoverflow.com/questions/61432488/php-error-unparenthesized-a-b-c-d-e-is-deprecated-use-either-a

Wrapping them in parentheses fixes it, but it wasn't an issue before the last update:

{{ ($model->exists ? 'Update' : 'Create') }}

@lukeraymonddowning
Copy link
Contributor Author

@mewejo yes. We now delegate to a method rather than handling in a ternary, so that issue is resolved with this PR or #37603

@taylorotwell taylorotwell merged commit 8699548 into laravel:8.x Jun 7, 2021
@lukeraymonddowning lukeraymonddowning deleted the stringable-improvements branch June 7, 2021 13:47
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