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

Fix bugs with latest plugin release 1.21.0 #2658

Merged
merged 12 commits into from Nov 4, 2022
Merged

Conversation

gchtr
Copy link
Member

@gchtr gchtr commented Oct 24, 2022

Resolves:

Issue

When the plugin version 1.21.0 of Timber was released, we accidentally included Twig 2.15.3 instead of Twig 1.44.7. This caused a lot of bugs for sites that auto-updated the Timber plugin (not recommended). We deactivated the 1.21.0 as the default release to disable that version from being found by auto-updates.

Solution

As a solution, this pull request makes sure that Twig version 1.44.7 is used when deploying the plugin version of Timber. When Timber is installed through Composer, the 2.x will still be installed.

We also added a new test configuration that tests PHP 8.0 and PHP 8.1 with version 1.44.7 of Twig by using prefer-lowest when installing Composer dependencies (thanks @nlemoine).

Unfortunately, I didn’t manage to get this fully working with PHP 8.1. A deprecation warning is somehow caught as content in the output in a shortcode test. I couldn’t find out how to "only" raise a PHP Deprecation warning. Does anybody have some ideas?

That’s why I’m hesitant about officially supporting PHP 8.1 in version Timber 1.x in the plugin version.

Impact

This pull request should fix the errors that people are seeing because we will revert to Twig 1.44.7:

Usage Changes

None.

Considerations

I already noted in the readme.txt that the new version should be 1.22.0. Though I’m not sure whether this qualifies as a hotfix and should be tagged with 1.21.1 instead. If guess if we require WordPress 5.3 as a minimum WordPress version, that wouldn’t qualify as a hotfix anymore.

Testing

Yes.

@mrfsrf
Copy link

mrfsrf commented Oct 24, 2022

"twig/twig": ">=1.44.7 || ^2.10",
This installs Twig v.3.x.x. I believe logical or should be omitted.

@gchtr
Copy link
Member Author

gchtr commented Oct 25, 2022

"twig/twig": ">=1.44.7 || ^2.10",
This installs Twig v.3.x.x. I believe logical or should be omitted.

@mrfsrf This installs Twig version 2, if you install Timber through Composer and the requirements are met. We’ve always allowed installing Twig version 2 this way.

For the plugin version of Timber, we explicitly request Twig 1.44.7, because we need to stay on version 1 for compatibility:

# Install the lowest compatible version of Twig.
composer update twig/twig:1.44.7 --no-dev

Timber v2 will allow you to install Twig 2 or 3.

Copy link
Member

@nlemoine nlemoine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, this example shows we should always run tests against the entire dependency range: meaning that prefer-lowest should be part of the matrix (for 2.x too).

bin/deploy-to-wp-org.sh Outdated Show resolved Hide resolved
@gchtr
Copy link
Member Author

gchtr commented Oct 25, 2022

BTW, this example shows we should always run tests against the entire dependency range: meaning that prefer-lowest should be part of the matrix (for 2.x too).

Yes, definitely!

@gchtr gchtr marked this pull request as ready for review October 26, 2022 06:21
@gchtr gchtr requested a review from jarednova as a code owner October 26, 2022 06:21
@jarednova
Copy link
Member

Thanks so much @gchtr and @nlemoine. I was writing a long question about the 8.1 and the failing test, I see the notes in readme.txt so helpfully answer it already! While much ink could be spilled in regards to 1.21.1 vs. 1.22.0, I think it's good as-is. Will merge and then proceed with WP.org deploying

@jarednova jarednova merged commit e11f42f into master Nov 4, 2022
@jarednova jarednova deleted the 1.x-prefer-lowest branch November 4, 2022 00:58
@xavivars
Copy link
Contributor

"twig/twig": ">=1.44.7 || ^2.10",
should be
"twig/twig": "^1.44.7 || ^2.10",

otherwise, twig 3 is installed (as it's > than 1.44.7)

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.

None yet

5 participants