BC break: narrowed return type of UploadControl in 3.2.4

[jankonas]

Version: 3.2.4

The return type of \Nette\Forms\Controls\UploadControl::getValue() was narrowed to FileUpload|array|null in 3.2.4 which breaks compatibility with custom controls extending UploadControl.

If the custom control returns something else than FileUpload or it's return type is mixed, PHP throws an error:

PHP Fatal error:  Declaration of CustomControl::getValue(): ... must be compatible with Nette\Forms\Controls\UploadControl::getValue(): Nette\Http\FileUpload|array|null

Could you please widen the return type again or is extending UploadControl not supported?

[dg]

This is, of course, an unintended BC break. Couldn't you fix your CustomControl?

[jankonas]

For now I "fixed it" by declaring conflict with nette/forms 3.2.4 in composer.json.

I don't think I have any option to fix it in other way then duplicate code from UploadControl instead of extending it, because the FileUpload is final so I can't make the return type to exted FileUpload.

The use case is that I have a javascript image cropper which uploads image with other data for cropping (coordinates, width and height) and the control adds this data to the value:

class CroppedImageControl extends UploadControl
{
	private ?CropperImageOptions $cropperOptions = null;


	public function __construct(?string $label = null)
	{
		parent::__construct($label);
		$this->monitor(Form::class, function (Form $form): void {
			$form->addHidden($this->name . 'CropperX')->setOmitted();
			$form->addHidden($this->name . 'CropperY')->setOmitted();
			$form->addHidden($this->name . 'CropperHeight')->setOmitted();
			$form->addHidden($this->name . 'CropperWidth')->setOmitted();
		});
	}


	public function getValue(): CroppedImage
	{
		/** @var FileUpload $file */
		$file = $this->value;

		/** @var array<string, string> $values */
		$values = (array) $this->form->getHttpData();

		$name = $this->getName() ?? '';

		return new CroppedImage(
			$file,
			(float) $values[$name . 'CropperX'],
			(float) $values[$name . 'CropperY'],
			(float) $values[$name . 'CropperHeight'],
			(float) $values[$name . 'CropperWidth'],
			$this->cropperOptions ?? new CropperImageOptions()
		);
	}


	public function setOptions(int $viewMode, string $dragMode, ?string $aspectRatio = null, ?string $data = null): void
	{
		$this->cropperOptions = new CropperImageOptions($viewMode, $dragMode, $aspectRatio, $data);
	}
}

[dg]

Of course duplicate the code, there's nothing wrong with that. It avoids a lot of problems.

[jankonas]

OK, just to confirm I understand you correctly, you want to keep the narrowed return type in the 3.x versions?

[dg]

Yes. I understand that it broke your code, but using inheritance was wrong in this case, it violated LSP. So the correct solution is to get rid of inheritance and that will solve the problem.

[forgie1]

Isn't ensuring compliance with LSP the responsibility of the subclass, as long as the parent class allows it? So why use such a drastic approach if it ultimately depends on the subclass?