zendframework / zend-form

Form component from Zend Framework
BSD 3-Clause "New" or "Revised" License
69 stars 87 forks source link

Passing options to the element factory from the form factory #138

Open MidnightDesign opened 7 years ago

MidnightDesign commented 7 years ago

Is it deliberate that there are no options passed from Zend\Form\Factory#create() to the FormElementManager?

I tried to add the options key from the $spec array to the FormElementManager#get() call, but that resulted in FactoryTest#testCanCreateWithConstructionLogicInOptions() being broken.

boesing commented 7 years ago

The problem is, that the Factory is not being automatically passed by the FormElementManager as it does in AnnotionBuilder respectively FormElementManager::injectFactory.

Therefore, a fresh Factory is being used Zend\Form\Collection:318.

MidnightDesign commented 7 years ago

Here's a short example of what I'm trying to do:

class MyForm extends Zend\Form\Form
{
    public function init()
    {
        parent::init();
        $this->add(['type' => MyFieldset::class, 'options' => ['foo' => 'My Option']]);
    }
}
class MyFieldset
{
    private $foo;
    public function __construct(string $foo)
    {
        $this->foo = $foo;
    }
}
class MyFieldsetFactory
{
    public function __invoke(
        ContainerInterface $container,
        $requestedName,
        array $options = null
    ) {
        return new MyFieldset($options['foo']); // $options is an empty array
    }
}

The problem is that Zend\Form\Factory doesn't pass the options to the FormElementManager.

Before Service Manager 3, my factory implemented MutableCreationOptionsInterface and I did return new MyFieldset($this->getOptions()['foo']);. That used to work.

kynx commented 7 years ago

@MidnightDesign I've just run into something similar when creating custom elements and ended up at the same line of code you highlighted. The options are passed, but to setOptions() not the constructor.

If you override setOptions() (and call that from the constructor) it should work. However you probably want to make the signature the same as the parent, or PHP7 will complain. And note it's called after the init() method has been called, so isn't much use if you want to pass options down to sub-elements from that (which I was trying to do, grrr...).

levofski commented 6 years ago

I encountered this when upgrading from 2.8.1 to 2.11.0

Unless I am missing something, surely the solution is to pass the 'options' part of the $spec array into the factory call

    public function create($spec)
    {
        $spec = $this->validateSpecification($spec, __METHOD__);
        $type = isset($spec['type']) ? $spec['type'] : Element::class;
        $creationOptions = isset($spec['options']) ? $spec['options'] : [];

        $element = $this->getFormElementManager()->get($type, $creationOptions);

        if ($element instanceof FormInterface) {
            return $this->configureForm($element, $spec);
        }
daslani commented 6 years ago

I'm looking to see if there's a particular reason why invocation of the getFormElementManager doesn't pass options if $spec contains options key and Factory waits until after object initializes to configure form. Other parts of the repo extending AbstractPluginManager pass options if available, seems only here this is omitted and prevents parts of my form construct or init to load. Adding $spec['options'] ?? null [PHP7] works. Any insight would be greatly appreciated.

    public function create($spec)
    {
        $spec = $this->validateSpecification($spec, __METHOD__);
        $type = isset($spec['type']) ? $spec['type'] : Element::class;

        $element = $this->getFormElementManager()->get($type, $spec['options'] ?? null);

        if ($element instanceof FormInterface) {
            return $this->configureForm($element, $spec);
        }

        if ($element instanceof FieldsetInterface) {
            return $this->configureFieldset($element, $spec);
        }

        if ($element instanceof ElementInterface) {
            return $this->configureElement($element, $spec);
        }

        throw new Exception\DomainException(sprintf(
            '%s expects the $spec["type"] to implement one of %s, %s, or %s; received %s',
            __METHOD__,
            ElementInterface::class,
            FieldsetInterface::class,
            FormInterface::class,
            $type
        ));
    }

Original block:

https://github.com/zendframework/zend-form/blob/4419eef6dbe9d276e7e27c6a25f022be74534959/src/Factory.php#L117

weierophinney commented 4 years ago

This repository has been closed and moved to laminas/laminas-form; a new issue has been opened at https://github.com/laminas/laminas-form/issues/18.