Fix compatibility with component-model 4.0#72
Conversation
For some reason, it was introduced escaped in 23a0df9, which is not supported and would fail with: sh: line 1: [@lint,: command not found The proper format is using JSON arrays: https://getcomposer.org/doc/articles/scripts.md#referencing-scripts
|
Bisected the failure to nette/component-model@b4e82e1 |
2a7a997 to
7c5a3df
Compare
davidsolc-ai
left a comment
davidsolc-ai
left a comment
There was a problem hiding this comment.
Thanks for adding component-model 4 compatibility. Please preserve the existing protected attached() extension point so this can remain a backward-compatible v3.0.1 fix. FormsReplicator 3.0 already permits nette/forms ^3.2.3, which includes Forms 3.3 and therefore component-model 4 on PHP 8.3+, so the incompatibility is a bug inside the currently advertised dependency range.
Nette requires explicit monitor() callbacks, but the callback may still be the existing protected method. Renaming it to a private method silently stops subclasses overriding attached() from receiving these events, even on component-model 3.x. The two inline suggestions retain that surface while removing the obsolete parent::attached() call.
Could you also add a regression test with a Container subclass that overrides attached(), calls parent::attached(), and records the callback? The existing PHP 8.3/8.4 highest jobs already verify Forms 3.3 with component-model 4.0.1.
| $this->monitor(Nette\Forms\Form::class, $this->componentAttached(...)); | ||
| $this->monitor(Nette\Application\UI\Presenter::class, $this->componentAttached(...)); |
There was a problem hiding this comment.
Use the existing protected method as the explicit callback. This follows component-model 4’s required monitor() API without dropping FormsReplicator’s subclass hook.
| $this->monitor(Nette\Forms\Form::class, $this->componentAttached(...)); | |
| $this->monitor(Nette\Application\UI\Presenter::class, $this->componentAttached(...)); | |
| $this->monitor(Nette\Forms\Form::class, $this->attached(...)); | |
| $this->monitor(Nette\Application\UI\Presenter::class, $this->attached(...)); |
There was a problem hiding this comment.
Done. Though this triggers PHPStan method.deprecation rule so had to suppress it.
| * Magical component factory | ||
| */ | ||
| protected function attached(Nette\ComponentModel\IComponent $obj): void | ||
| private function componentAttached(Nette\ComponentModel\IComponent $obj): void |
There was a problem hiding this comment.
Please retain the protected method. It has been subclass-visible since 2019 and is not marked @internal; making the replacement private creates an avoidable BC break. With the explicit callbacks above, no parent::attached() call is needed and this remains compatible with both component-model 3 and 4.
| private function componentAttached(Nette\ComponentModel\IComponent $obj): void | |
| protected function attached(Nette\ComponentModel\IComponent $obj): void |
There was a problem hiding this comment.
Renamed back and added PHPStan deprecation suppression instead. Though we should probably deprecate using attached in subclass as well in favour of custom monitor calls.
component-model 4.0 removes the use of `Nette\SmartObject` so we can no longer access the private properties: nette/component-model@c07bbee
Calling monitor handlers has been deprecated since component-model 3.0: nette/component-model@857a493 And has been removed in 4.0: nette/component-model@71321c9 Let’s remove the deprecated parent method call. This is fine since in the parent class chain: - `Nette\Forms\Container` - `Nette\ComponentModel\Container` - `Nette\ComponentModel\Component` only the last one ever had the `attached` method and it was always empty.
Calling monitor with implicit handlers has been deprecated since component-model 3.0: nette/component-model@857a493 And has been removed in 4.0: nette/component-model@71321c9 Let’s switch to explicit handler.
The method has been deprecated since component-model 3.0: nette/component-model@857a493 And has been removed in 4.0: nette/component-model@71321c9 Let’s ignore the deprecation warning reported by PHPStan. This occured on PHP 8.2 when installing highest dependencencies.
PHPStan is confused by stubs, and infers that `Container::getComponents()` returns an untyped `array`. Perhaps because the stubs claim to return `Iterator` while upstream returns an `IComponent[]`. phpstan/phpstan-nette#141
I would rather just drop that feature so I do not want to bother. |
nette/forms 3.3.0 add support for component-model 4.0. component-model 4.0 seems to have fixed the issue by changing the return type hint to `array`: nette/component-model@c40aa8e We still need to keep the ignores for compatibility with older versions.
We are passing a function that has extra arguments compared to the `callable` in `extensionMethod` parameter type. Since a function with extra arguments is a supertype of a function without them, not a subtype, it will be rejected as per contravariance of arguments rule. See nette/forms#354 Let’s ignore it for now.
After the dependency bump, the tests started to fail:
Nette\InvalidStateException: Component 'users' is not attached to 'Nette\Forms\Form'.
1. tests/Replicator/ContainerTest.php:376
Tester\TestCase->run()
2. vendor/nette/tester/src/Framework/TestCase.php:53
Tester\TestCase->runTest()
3. vendor/nette/tester/src/Framework/TestCase.php:132
KdybyTests\Replicator\ContainerTest->testAddContainer()
4. tests/Replicator/ContainerTest.php:257
KdybyTests\Replicator\BaseForm->addDynamic()
5. tests/Replicator/ContainerTest.php:350
Nette\Forms\Container->offsetSet()
6. vendor/nette/component-model/src/ComponentModel/ArrayAccess.php:27
Nette\Forms\Container->addComponent()
7. vendor/nette/forms/src/Forms/Container.php:299
Nette\ComponentModel\Container->addComponent()
8. vendor/nette/component-model/src/ComponentModel/Container.php:76
Nette\ComponentModel\Component->setParent()
9. vendor/nette/component-model/src/ComponentModel/Component.php:181
Nette\ComponentModel\Component->refreshMonitors()
10. vendor/nette/component-model/src/ComponentModel/Component.php:230
Kdyby\Replicator\Container->attached()
11. src/Replicator/Container.php:105
Kdyby\Replicator\Container->loadHttpData()
12. src/Replicator/Container.php:221
Nette\Forms\Container->getForm()
13. vendor/nette/forms/src/Forms/Container.php:330
Nette\ComponentModel\Component->lookup()
14. vendor/nette/component-model/src/ComponentModel/Component.php:67
Bisected that to nette/component-model@b4e82e1
2 participants
No description provided.