Fixes #3 - Element::init is called before Element::setOptions when using Factory but not when using FormElementManager

[froschdesign]

Q	A
Documentation	no
Bugfix	yes
BC Break	no
New Feature	no
RFC	no
QA	no

Description

Fixes #3: Element::init is called before Element::setOptions when using Zend\Form\Factory but not when using FormElementManager

[Xerkus]

Hold off merging this. Implications need to be understood.

I remember that creating form via spec is not compatible with custom forms (or elements) that rely on init for self-configuration. Form configuration via spec is meant for generic forms and those do not have init().

[froschdesign]

@Xerkus
Unfortunately, this also affects the form builders, which is why I stumbled over it.
But rewriting the elements so that the factory creates them correctly cannot be the solution.

[Xerkus]

Laminas\Form\Factory (Factory) modifies state of objects returned by FormElementManager (manager).
Element::init() is a part of manager creation flow. Since Factory performs its logic after manager none of it can be used in initializer.
For that reason initializer can only depend on what is provided by manager factory or delegators.

laminas-servicemanager plugin manager provides build() which allows to create new instance with specified build parameters which are passed to the plugin manager factories as third argument. So as long as factory supports that... Default InvokableFactory does not but FormElementManager is configured with ElementFactory and it uses ElementFactory for "invokable" from configuration. That leaves factories explicitly defined by users.

One significant difference of build() as compared to get() is that it entirely ignores shared service setting and always creates new instance. Change here is a potential BC break. However FormElementManager is set as not shared by default. If create spec does have options or they are empty the potential impact is further mitigated.

Overall change here makes sense. I am rather inclined to treat this as a new feature rather than a bug fix. Options are already set as they meant to.

I am conflicted about this somewhat because of a surprise factor. Documentation is already pretty unclear or even misleading. With this change spec options become available for init() but nothing else does. Worse, if those options are set via manager factory and then overridden in init() the Factory would proceed to set them a second time as the very last action. I was worried about setting options twice but it does not seem to have any other negative effect.

As a followup of slack discussion I tried to identify needed documentation improvements only some of which should be handled here:

• clearly state that Factory is entirely different and unrelated to Manager factories. Any references in the documentation to either must make that distinction clear.
• clearly state that Factory performs its configuration based setup on an instance of Form/Fieldset/Element after Manager is done creating them. That includes manager factories, delegators and initializers. Conversely everything outlined in the spec is not available for manager factory, delegator or init().
• for this PR clearly state that $spec['options'] are now passed to FormElementManager and in turn to manager factory. User defined manager factory must accept and pass along those options for them to be available during init(). See ElementFactory.
• for this PR clearly state that Factory will apply options again after everything else, potentially overriding anything set in init()
• optionally advise users as a best practice to stick to either approach where possible for a single source of truth.
• update all examples of configuring forms, fieldsets and elements via constructors to explicitly inject Factory as a direct constructor dependency. It is very unclear when Factory comes into play leading to further confusion.
• Update initializers section to explain how Factory comes into play. Essentially, it is injected as dependency by another initializer and it is used when methods on the form/fieldset are used to attach validators, filters and child elements. See above point. It is not used by Manager to create or configure object on which init() is called.

[Xerkus]

Build parameters must be ['options' => $spec['options']] according to ElementFactory and if options are not set or empty build parameters should be null to not trigger plugin manager's build() behavior.

ElementFactory also accepts element name as build parameters, so we should probably pass it too if $spec has it.

Something along those lines:

Suggested change

	$element = $this->getFormElementManager()->get($type, $spec['options'] ?? []);
	$buildParams = null;
	if (! empty($spec['options'])) {
	$buildParams = ['options' => $spec['options']];
	$name = $spec['name'] ?? null;
	if ($name !== null && $name !== '') {
	$buildParams['name'] = $name;
	}
	}
	$element = $this->getFormElementManager()->get($type, $buildParams);

[Xerkus]

note that options could be iterable, according to code. I did not handle it in suggested change.