gremo / ZurbInkBundle

Creating email templates is hard. This Symfony Bundle provides help.
MIT License
24 stars 12 forks source link

PHP 7.2 makes an error while using Inky #1

Closed cierzniak closed 6 years ago

cierzniak commented 6 years ago

Hi, I've already updated my stack to PHP 7.2 and every time I try to send email which is generated by this bundle I've got:

Uncaught PHP Exception Twig_Error_Runtime:
"An exception has been thrown during the rendering of a template
("Warning: count(): Parameter must be an array or an object that implements Countable")."
at /var/www/vendor/gremo/zurb-ink-bundle/src/Resources/views/FoundationForEmails/2/base.html.twig
 line 44

Any tip what could be wrong?

Package locked at ^2.3 in composer.json

gremo commented 6 years ago

Hi @cierzniak I'll make a test with PHP 7.2. In the meanwhile, can you make a quick test with master version?

cierzniak commented 6 years ago

Got dev-master (6dca869) and error is still the same:

Twig_Error_Runtime: An exception has been thrown during the rendering of a template ("Warning: count(): Parameter must be an array or an object that implements Countable").

at vendor/gremo/zurb-ink-bundle/src/Resources/views/foundation-emails/base.html.twig:23

My templates/_emails/base.html.twig:

{% extends '@GremoZurbInk/foundation-emails/base.html.twig' %}

{% block stylesheets %}
    {{ parent() }}
    {{ zurb_ink_add_stylesheet('../templates/_mails/main.css') }}
{% endblock %}

{% block content %}
    <table align="center" class="container header float-center">
        <tbody><tr>
            <td>
                <table class="row collapse">
                    <tbody><tr>
                        <th class="small-6 large-4 columns first last">
                            {{ macro.asset_image('images/logo.png') }}
                        </th>
                    </tr></tbody>
                </table>
            </td>
        </tr></tbody>
    </table>
    <table align="center" class="container body-drip float-center">
        <tbody><tr>
            <td>
                {% block emailContent %}{% endblock %}
            </td>
        </tr>
        </tbody>
    </table>
{% endblock %}

$php -v:

PHP 7.2.3-1+ubuntu16.04.1+deb.sury.org+1 (cli) (built: Mar 6 2018 11:18:25) ( NTS ) Copyright (c) 1997-2018 The PHP Group Zend Engine v3.2.0, Copyright (c) 1998-2018 Zend Technologies with Zend OPcache v7.2.3-1+ubuntu16.04.1+deb.sury.org+1, Copyright (c) 1999-2018, by Zend Technologies with Xdebug v2.6.0, Copyright (c) 2002-2018, by Derick Rethans

gremo commented 6 years ago

Thanks, so I'ts confirmed, I'll take a look at it.

cierzniak commented 6 years ago

There is no problem while using

{% extends '@GremoZurbInk/base.html.twig' %}

but without inky my email looks bad and I have to spend few hours to edit bunch of them

gremo commented 6 years ago

It seems to me an issue related https://github.com/paquettg/php-html-parser which is being used by Inky library (abandoned). I'll try to find out a way to avoid the error.

PHP 7.2 introduce this incompatible change socount('string') raise a warning in Selector.php.

cierzniak commented 6 years ago

Any updates? Did you see this: https://github.com/paquettg/php-html-parser/pull/126 ?

tchapi commented 6 years ago

Any updates on this issue by any chance ? Thanks

gremo commented 6 years ago

Sorry for the late response guys.

Here is the situation: as pointed by @cierzniak the fix was merged (https://github.com/paquettg/php-html-parser/pull/126) but in the dev branch. As soon it will be merged into master and released, problem should gone.

But I don't really know when a new tag will be released, the project seems quite "dead".

By the way, maybe you could use this fork https://github.com/thesoftwarefanatics/php-html-parser instread of the original library?

tchapi commented 6 years ago

Thanks for your answer @gremo !

Even if this is not ideal, your solution works :

"require": {
    "php": "^7.1",
    "gremo/zurb-ink-bundle": "^2.3",
    "thesoftwarefanatics/php-html-parser": "^1.8",
    [ ... ]
},

It is a good workaround for now

davidromani commented 6 years ago

Maybe it can help if all of us, claim this requirement here

https://github.com/paquettg/php-html-parser/commit/1fd97411e0dc042c221263c61bd393973efb9c93

GenieTim commented 6 years ago

As this repo is a Symfony Bundle, would it be a rational step to refactor to use the Symfony DOM Crawler component one day?

Edit: sorry, I just realized that the Inky component is dependent on the HTML Parser, not this bundle. So no, as long as the Inky component is a dependency and not integrated, it does not make sense.

solverat commented 6 years ago

Hey guys. Just here to say: We just successfully moved away from hampe/inky to lorenzo/pinky with ease. It's small and quite smart! It also has no dependency and works with php 7.2. 🎉

GenieTim commented 6 years ago

@solverat cool! Will there be a PR into this repo? Or did you achieve the move on other levels?

gremo commented 6 years ago

@solverat I din't switched to pinky to maintain compatibility with PHP 5.3, but feel free to make a PR, I'll review it.

solverat commented 6 years ago

I'm on a different project and stumbled into this while searching for alternatives. :) It's really a 5 minute work:

Composer

replace

"hampe/inky": "^1.3.6"

with

"lorenzo/pinky":"1.0.1" // or if you're a risk lover: "^1.0"

HtmlUtils

Replace this line: https://github.com/gremo/ZurbInkBundle/blob/cf655b96f22563ae24d3e1067e7700221349a643/src/Util/HtmlUtils.php#L58-L61

with:

use Pinky;

// [...]

public function parseInky($content)
{
    return Pinky\transformString($content)->saveHTML();
}

Done! Good Luck to you guys, greets from Austria!

gremo commented 6 years ago

New version v4.0.0 uses lorenzo/pinky!

GenieTim commented 6 years ago

Thank you very much. I recommend updating the README for future users to see what tags to use for which PHP version?

cierzniak commented 6 years ago

Thank you for support :wink: