You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
To further improve SA and minimise the number of checks for "Do I have a renderer yet", either make the renderer a constructor dependency for HelperPluginManager (And provide a factory) or throw an exception on retrieval if null, effectively making the signature for HelperPluginManager::getRenderer(): RendererInterface and the signature for AbstractHelper::getView(): RendererInterface(i.e. enforce a return type) … It would also be a good idea to update the signature for HelperInterface::getView()!
Background
This was recently surfaced in slack because until recently, the documented return type for AbstractHelper::getView() was changed from RendererInterface to RendererInterface|null(The latter being what it actually is). Consumers extending AbstractHelper now have errors in their SA tools.
Guaranteeing a renderer for both of these will likely solve a large number of possible type errors and make the public api of the package easier to work with.
Considerations
There will be a number of laminas packages affected by this change that will require work to upgrade to 3 - at least i18n, flash-messenger, form and likely more.
The signature changes will potentially affect any consumer that has extended AbstractHelper - the impact is unlikely to cause to many problems unless they have re-implemented the getView() method which is unlikely
Proposal(s)
Change the docs for "Advanced Helper Usage" so that the examples of creating and registering helpers do not extend AbstractHelper providing examples of a simple invokable as well as a helper having constructor dependencies along with an example factory with instructions on registering plugins.
Change the return types for methods retrieving the underlying view renderer and make them exceptional
Update all callers of these methods, fix broken tests and introduce new tests to cover the changed behaviour
Mark AbstractHelper as @\internal to discourage extending it outside of the package
The text was updated successfully, but these errors were encountered:
RFC
Goal
To further improve SA and minimise the number of checks for "Do I have a renderer yet", either make the renderer a constructor dependency for HelperPluginManager (And provide a factory) or throw an exception on retrieval if null, effectively making the signature for
HelperPluginManager::getRenderer(): RendererInterface
and the signature forAbstractHelper::getView(): RendererInterface
(i.e. enforce a return type) … It would also be a good idea to update the signature forHelperInterface::getView()
!Background
This was recently surfaced in slack because until recently, the documented return type for
AbstractHelper::getView()
was changed fromRendererInterface
toRendererInterface|null
(The latter being what it actually is). Consumers extendingAbstractHelper
now have errors in their SA tools.Guaranteeing a renderer for both of these will likely solve a large number of possible type errors and make the public api of the package easier to work with.
Considerations
There will be a number of laminas packages affected by this change that will require work to upgrade to 3 - at least
i18n
,flash-messenger
,form
and likely more.The signature changes will potentially affect any consumer that has extended AbstractHelper - the impact is unlikely to cause to many problems unless they have re-implemented the
getView()
method which is unlikelyProposal(s)
AbstractHelper
providing examples of a simple invokable as well as a helper having constructor dependencies along with an example factory with instructions on registering plugins.The text was updated successfully, but these errors were encountered: