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

Should semantic version be followed in relation to breaking changes? #2654

Closed
mmcev106 opened this issue Oct 19, 2022 · 4 comments
Closed

Should semantic version be followed in relation to breaking changes? #2654

mmcev106 opened this issue Oct 19, 2022 · 4 comments

Comments

@mmcev106
Copy link

The twig update in 1.21.0 introduced some breaking changes, which caused an old site of mine to break with errors like the following:

Uncaught TypeError: Argument 1 passed to Twig\Environment::addFilter() must be an instance of Twig\TwigFilter, string given

They were easy to solve, but they made me wonder if any breaking change should have necessitated a version 2.0.0 of this plugin per semantic versioning. I'm guessing it's just not worth your time to be too strict about these things, which is totally fine. I was just curious, so I figured I'd ask.

@gchtr
Copy link
Member

gchtr commented Oct 20, 2022

Yes, of course, the idea is to follow semantic versioning. (That’s why we are still working on a version 2 that will introduce a lot of breaking changes.)

We didn’t mean to introduce a breaking change and are currently investigating why this happened. We might have shipped Twig v2 instead of Twig v1 in the plugin by mistake. We hope that we can release a fix for this, soon. Sorry for that.

Would you mind giving us a code example that you had to fix? A short snippet of a code before and after? We could include it in our tests, then.

@mmcev106
Copy link
Author

mmcev106 commented Oct 20, 2022

I'm not an expert on the site in question yet (it was created by someone else years ago), but I believe this is a distilled example of part of the code that broke:

add_filter('get_twig', function($twig){
	$twig->addFilter('myCustomFunction', new Twig_SimpleFilter('myCustomFunction', function($someArg){
		// Take some action
	}));
});

For all I know, that is incorrect usage of twig. It did work with the version of twig included in 1.20.0 though.

@mgruhn
Copy link

mgruhn commented Oct 27, 2022

Just jumping into this ticket to note that, in addition to the filter issue identified above, 1.21 also broke custom loaders. I had custom loader set via the timber/loader/loader filter, an implementation of \Twig\Loader\LoaderInterface. But with 1.21, twig's LoaderInterface definition changed (renaming getSource), so the sites with this code all raised PHP errors. I think this supports the idea that 1.21 included an inadvertent twig version update.

@gchtr
Copy link
Member

gchtr commented Nov 1, 2022

@mgruhn The reason for this error is also that Twig version 2.0 was mistakenly included in the plugin version of Timber when we ran an automated script to build the plugin. This shouldn’t be a problem with the next release of Timber, where the most current version of Twig 1.x will be included. The interface didn’t change there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants