Pushed breaking change in 4.2 release

[pimjansen]

Hey all,

I just noticed that version 4.2 contains a breaking change in a minor release. When i review this PR it states "Allow laminas form 3.0 while keeping BC".

For this release however an actual class implementation is renamed and replaced with another which breaks the implementation for all its consumers.

Kinda risky if you ask me to push this as a minor. I agree that the migration is fairly easy since the polyfill behaves the same way but technically this should be a major release?

Best

[driehle]

Could you please elaborate which class implementation has changed (i.e. please name the fully qualified name of that class) between 4.1 and 4.2 from your point of view?

[pimjansen]

The image shows it best i guess. the ObjectSelect has been renamed to a Polyfill. Same goes for the other select types i can see in the PR itself.

[driehle]

Well, you need to look at the whole PR. Though you mentioned the polyfills in your initial issue description, I am not sure if you are aware how polyfills actually work. Please have a look the the following change from the same PR:

First, the file autoload/polyfill-form-elements.php is included in the autoload section of composer.json. Therefore, this file will always be loaded without any conditions.

Second, the file checks which version of laminas/laminas-form the user has installed. That is the part with null === $reflectionMethod->getReturnType(). It will be true for 2.x and false for 3.x. You can look into a diff of laminas-form 2.x and 3.x to validate that this is actually the case.

Third, the class DoctrineModule\Form\Element\ObjectMultiCheckboxV2Polyfill is pushed into the right place when laminas-form 2.x is installed using class_alias(), i.e. it is aliased to DoctrineModule\Form\Element\ObjectMultiCheckbox. And that is exactly the name it had in earlier versions. Please read the PHP documentation on class_alias: https://www.php.net/manual/en/function.class-alias.php. This function creates an exact copy of the class, i.e. defining the class as ObjectMultiCheckboxV2Polyfill and aliasing it to ObjectMultiCheckbox is absolutely identical to defining it directly as ObjectMultiCheckbox!

So as DoctrineModule 4.1.x and 4.2.0 both provide DoctrineModule\Form\Element\ObjectMultiCheckbox with the very same signatures, there is no BC break in DoctrineModule 4.2.0.

---

There is only one case where an update from DoctrineModule 4.1.x to 4.2.0 can cause troubles: If you in your application use laminas-form and you do not have specified laminas-form in your composer.json, upgrading DoctrineModule to 4.2.0 will upgrade laminas-form from 2.x to 3.x in your application. And that will obviously result in issues, as laminas-form 3.x has BC breaks (that is why it is a new major version).

If that is the case I strongly recommend using a tool like compoer-require-checker which checks if you have so-called soft dependencies in your application.

Lastly, you refer to using polyfills as "kinda risky". I disagree with that statement. In laminas-form, which back then was still called zendframework/zend-form, the same principle was applied in 2.15.x. 2.14.x only supported servicemanager 2.x, 2.15.x supported servicemanager 2.x and 3.x through polyfills. They event went a step further, in 2.16.x they removed support for servicemanager 2.x. Considering that laminas-form (counting both laminas-form and zend-form) has over 16 million installs, I don not think that using polyfills is a risky behaviour.

[pimjansen]

I was not aware of those reflection methods tbh. Will review those usecases. Thanks for the swift response!