Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

Element constructor have different behaviours #63

Closed
Slamdunk opened this issue Apr 28, 2016 · 2 comments
Closed

Element constructor have different behaviours #63

Slamdunk opened this issue Apr 28, 2016 · 2 comments

Comments

@Slamdunk
Copy link
Contributor

Hi, #43 with #45 introduced an unexpected behaviour that I consider a BC break.

All starts by defining a custom element as invokable:

class My\Custom\Element extends Zend\Form\Element
{}

$formElementManager = $this->getServiceLocator()->get('FormElementManager');
$formElementManager->setInvokableClass('my_element', 'My\Custom\Element');

At this point, behavious are different depending on built-in or custom elements, and what I specify as options array key:

// Case 1
$element = $formElementManager->get('my_element', array(
    'name' => 'my_name',
    'my_option' => true,
));
// is equivalent to
$element = new My\Custom\Element(array(
    'name' => 'my_name',
    'my_option' => true,
));

var_dump(gettype($element->getName())); // array

Because it's handled by https://github.com/zendframework/zend-servicemanager/blob/release-2.7.6/src/AbstractPluginManager.php#L254

// Case 2
$element = $formElementManager->get('Zend\Form\Element', array(
    'name' => 'my_name',
    'my_option' => true,
));
// is equivalent to
$element = new Zend\Form\Element('my_name', array(
    'name' => 'my_name',
    'my_option' => true,
));

var_dump(gettype($element->getName())); // string

Because it's handled by https://github.com/zendframework/zend-form/blob/release-2.8.1/src/ElementFactory.php#L69

And this is going even harder to understand if we use the Zend\Form\Factory

// Case 3
$factory = new Zend\Form\Factory($formElementManager);
$element = $factory->create(array(
    'type' => 'my_element',
    'name' => 'my_name',
    'options' => array(
        'my_option' => true,
    ),
));
// is equivalent to
$element = new My\Custom\Element(array(
    'my_option' => true,
));
$element->setName('my_name');
$element->setOptions(array(
    'my_option' => true,
));

var_dump(gettype($element->getName())); // string

Because of https://github.com/zendframework/zend-form/blob/release-2.8.1/src/Factory.php#L204-L214

// Case 4
$factory = new Zend\Form\Factory($formElementManager);
$element = $factory->create(array(
    'type' => 'Zend\Form\Element',
    'name' => 'my_name',
    'options' => array(
        'my_option' => true,
    ),
));
// is equivalent to
$element = new Zend\Form\Element('element', array(
    'my_option' => true,
));
$element->setName('my_name');
$element->setOptions(array(
    'my_option' => true,
));

var_dump(gettype($element->getName())); // string

Without name

// Case 5
$factory = new Zend\Form\Factory($formElementManager);
$element = $factory->create(array(
    'type' => 'my_element',
    'options' => array(
        'my_option' => true,
    ),
));
// is equivalent to
$element = new My\Custom\Element(array(
    'my_option' => true,
));
$element->setOptions(array(
    'my_option' => true,
));

var_dump(gettype($element->getName())); // array

// Case 6
$factory = new Zend\Form\Factory($formElementManager);
$element = $factory->create(array(
    'type' => 'Zend\Form\Element',
    'options' => array(
        'my_option' => true,
    ),
));
// is equivalent to
$element = new Zend\Form\Element('element', array(
    'my_option' => true,
));
$element->setOptions(array(
    'my_option' => true,
));

var_dump(gettype($element->getName())); // string

The transition fro SMv2 to SMv3 is irrelevant, because Zend\ServiceManager\Factory\InvokableFactory acts exactly like Zend\ServiceManager\AbstractPluginManager::createFromInvokable.

The sources of misunderstanding are:

  1. The name acts differently in different contexts, inside or outside options, through Factory or not
  2. Built-in plugins use ElementFactory to face [1], while custom invokables are instantiated naturally
  3. The constructor may behave weird when inherited by Zend\Form\Element and not overwritten

The fact is, if name and options are required, the constructor must be declared in the interface.
If name and options are optional, Zend\Form\Element must not have a constructor at all.

In my humble opinion, we should decide to be clear about this topic, and:

  1. Remove the constructor from Zend\Form\Element
  2. Lazy-load Zend\Form\Fieldset::$iterator [PriorityList]
  3. Remove the new Zend\Form\ElementFactory (in favour of createFromInvokable or InvokableFactory depending on SM version)
  4. Document that FormElementManager only instantiate the object, and configuration must be done with the Factory

So

  1. Built-in elements will behave like always
  2. Custom elements extended by Zend\Form\Element without constructor will behave like build-in elements
  3. Custom elements extended by Zend\Form\Element with constructor will receive options array
  4. Custom elements implementing Zend\Form\ElementInterface with constructor will receive options array

With no more array-name.

What do you think?

@weierophinney
Copy link
Member

What do you think?

I think it's a massive BC break, to be honest.

  • Removing the constructor breaks any class that was relying on the functionality it provided.
  • InvokableFactory does not behave exactly like AbstractPluginManager::createFromInvokable(). Under v2, if you add an invokable mapping for 'foo' => 'Zend\Form\Element', an instance of the latter will be created, and mapped to the service 'foo'. However, with v3, mapping 'foo' => InvokableFactory::class will fail as the class 'foo' does not exist; you need to instead alias 'foo' to Zend\Form\Element, and then add a factory mapping for Element::class => InvokableFactory::class. The changes from 2.7.0 forward are meant to make the component forwards-compatible with that paradigm.

I think I found the root issue, however: it was due to the fact that 2.7.0 changed the order in which the default initializers were registered, pushing them to the top of the initializer queue, instead of the end. #65 addresses that.

@Slamdunk
Copy link
Contributor Author

Under v2, if you add an invokable mapping for 'foo' => 'Zend\Form\Element', an instance of the latter will be created, and mapped to the service 'foo'. However, with v3, mapping 'foo' => InvokableFactory::class will fail

Why should you change the map? In v3, if you keep the v2 'foo' => 'Zend\Form\Element' within the invokables, you have the exact behaviour you explained, as far as I can read in the SM migration guide.

I read your #65 and the main issue is still there. I'll try to simplify the topic.

If

  1. You have a custom element
  2. You do not specify the name
  3. You do not use the new ElementFactory

The result is a mess because you have an array as the element name. How #65 helps with this scenario?

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

No branches or pull requests

2 participants