After update to 2.17, it breaks sites on PHP 7.4

The MR fixes this issue.

However, introducing breaking changes in a minor version (happening here and in slick) is terrible. This breaks dozens of our websites and we now have to go through a fix them all in unscheduled context.

An obvious overlook, thank you. I got no more PHP 7.4 for quite a while.

Unfortunately, coder and drupal-check do not help us with BC, trailing dot or comma so far.
They normally warned us to put dot, but never warned us to remove it :)

> introducing breaking changes...This breaks dozens of our websites
May I know what breaks to fix in the next releases?

I tested 2.17 breaking changes with my own old sites before releasing it, and found no real regressions, nor breaking changes, at least so far. So your use cases would be helpful to minimize the issues even more.

Thank you.

With this version of Blazy and using PHP 7.4, you need to make modifications in the toContent function located in blazy/src/internals/Internals.php with the following code:

public static function toContent(array &$data, $unset = false, array $keys = ['content', 'box', 'slide']) {
    $result = [];
    foreach ($keys as $key) {
        $value = $data[$key] ?? $data["#$key"] ?? [];
        if ($value) {
            $result = $value;
            break;
        }
        if ($unset) {
            unset($data[$key]);
        }
    }
    return $result;
}

Unfortunately, coder and drupal-check do not help us with BC, trailing dot or comma so far.

I guess you would have to run the tests with PHP 7.4 to get the syntax error.

May I know what breaks to fix in the next releases?

We have sliders like these on an old site (hence still on PHP 7.4) which looks like this:

After the update, the 3 dots to page through the slides turned into numbers 1-3 with the premium colour as the background and arrange top down instead of left top right.

When I spotted that, I went to the release page to see if I could quickly find a reference to a change that may explain that. What I found was a note, that this release contains breaking changes. When I saw that, and since I didn't have spare time to do proper research, I decided to go back to the previous releases of Blazy and Slick. We will look into it later. I'm sure, this is most likely some markup change and/or classes that have changed or got removed, so that CSS is screwed.

> have to run the tests with PHP 7.4 to get the syntax error.
Good idea, thanks. Drupal deprecated its tests for Gitlab CI, and I removed all Drupal tests for Gitlab since then. I'll see if I can find Gitlab options for PHP4 like Drupal's, or re-enact Drupal tests specific for old PHP.

> the 3 dots to page through the slides turned into numbers 1-3
Hm, nothing that I am aware of would cause this issue, except unsupported Slick versions, check out:
https://www.drupal.org/project/slick#slick-requirements →

The supported are 1.6+ and <= 1.8.0. With faking 1.8.0 noted there:
> Important! If any issues with specific number 1.8.0, be sure version in Slick package.json matches the version written in slick.js. The reason, release 1.8.1 with package.json 1.8.1 has also version 1.8.0 written in slick.js as of this writing 2021/10. If they don't match, they are not supported by this module.

That inconsistent number was newly found 2 or 3 years ago when working with breaking changes of Accessible Slick.
Solutions: try matching your versions to the supported ones. If persists, please create the issue in Slick.

> about your patch:
I am sorry for missing you the credit.

Drupal does normally put the credit automatically in the commit message below when I copied, but apparently not when not using manual patches.

Thank you.

Thanks @gausarts, I'll report back as soon as I know more.

Testing with GitLab CI is amazing for this. Look out for the `parallel` instructions. I believe there are even some comments in the template. That allows you to define a kind of matrix with versions of PHP, DB, Drupal, etc. with which you want to run tests.

Thanks for the hint with Slick. Just checked in that old project where we use 1.8.0 indeed. Not sure about all our other projects, but I'll keep that in mind.

With regard to credits, there is no automatic assignment anymore, since a lot of users abused that by just uploading some fake files to receive credits for nothing.

> where we use 1.8.0...
Be sure to view its package.json, in browsers and the version matches its slick.min.js as well. Due to faking/ misleading number 1.8.0 in slick.min.js while its package.json says 1.8.1. That is the trouble version.

It is a small complicated thing to explain issue, that is why, to put it simply, 1.8.1+ is not supported due to such complications.
But there are issues in Slick with @Anybody's patches, IIRC, and decent explanations about dots, etc.

Confirming this bug and how highly disruptive it is.

[error]  ParseError: syntax error, unexpected ')', expecting variable (T_VARIABLE) in Composer\Autoload\{closure}() (line 548 of /tmp/test-general-installability/web/modules/contrib/blazy/src/internals/Internals.php) #0 /tmp/test-general-installability/vendor/composer/ClassLoader.php(427): Composer\Autoload\{closure}('/tmp/test-gener...')

2.18 was scheduled here:
https://git.drupalcode.org/project/blazy/-/blob/8.x-2.x/docs/CHANGES.md?...

I apologized for inconveniences.

No problem. Things happen. Definitely no need to apologize!

But … why wait until September 24? 😅 Better to unblock people ASAP. Releases are cheap, no harm in doing plenty of 'em!

That is contradictory to Drupal docs, but sound more convincing.

I was doctrinized to not release too often, visit open issues, fix them, ensure no opened bugs, etc.
Shortly take your time. I forgot the link, but someone who wrote it must remember.

Here is the link to prove that such doctrine was printed in my mind before I removed it in 2.17:
https://git.drupalcode.org/project/blazy/-/blob/8.x-2.17-rc5/docs/CHANGE...

> Please bear with frequent releases, it was for sub-modules tests. Their
tests help spot many regressions, reducing one at a time every releases.
We normally release some 3 months or years periods. It is special for 3.x.

First time I hear that! 🤯😅

Quoting Drupal core committer @larowlan:

Dear #drupal module maintainers.
If you merge a bugfix, cut a release. Releases are cheap. Don't make people switch to dev or apply patches. If you're not going to cut a release because you're waiting for an extra issue, ie to bundle a couple of fixes, point people to that issue so they can help get it done and get the release out faster.
In npm land many use semantic-release which auto creates a bugfix release for every push to main. We need to be mimic this.
Release early, release often
Thanks

— https://drupal.community/@larowlan/110529857776420376

Quoting Drupal core subsystem maintainer @gabesullice:

[…]

Please publish a stable release.

— https://www.sullice.com/posts/2021/07/08/please-publish-a-stable-release/ + https://twitter.com/GabeSullice/status/1413164106996191235 +

@jurgenhaas, thank you for contributions.

@Wim Leers, thank you for a little enlightenment, and liberation. I should no longer feel guilty for frequent releases as said #15 from now on.

I intentionally left open after commit to avoid dups while attending another open bugs.

Housekeeping for 2.18.
Thank you.

Hah, happy to "liberate" you! 😄

Happy shipping! 🚢

Automatically closed - issue fixed for 2 weeks with no activity.