Head* and InlineScript helper subclasses no longer supported; need best practice for customization

[demiankatz]

BC Break Report

Q	A
Version	2.29.0 or higher

Summary

My application has been using laminas-view for many years, and part of our implementation relies on subclassing the laminas-view Head* and InlineScript helpers to integrate with our theme and asset pipeline systems (for example, here is our HeadLink helper). I notice that as of laminas-view 2.29.0, the helpers have been marked @final, so my subclasses are triggering static analysis errors. It also appears that rewriting my code to proxy the helpers instead of extending them is also impractical, since some of the methods we would need are marked @internal and may not remain public indefinitely.

I realize that for now this is just an annotation, so it is not a true BC break (apart from upsetting my static analysis tool). But I assume it is a sign of things to come for the 3.x line, and I want to get an understanding of expected best practices so I can keep my software working. I was wondering if anyone could share insight into why the decision was made to make extending the classes impossible, or what the future plans are for laminas-view. Before I invest time into rewriting all of my view helpers, I want to be sure I have the best strategy in mind that will allow me to continue using the library going forward.

As I can see it, I have a few options:

1.) Copy and paste all the code, and modify the copied code. This is obviously a bad idea for a number of reasons, but as far as I can tell, it may be the only approach guaranteed not to break. 2.) Ignore the @final annotations, tell static analysis to ignore the problem, and continue extending the class. This is also obviously a bad idea, and it’s an even worse idea if the “final” eventually moves out of the annotations and into the actual PHP code (which I presume it probably will as part of the 3.x upgrade). 3.) Rewrite my helpers as proxies. This is probably the most architecturally sound approach, but if we lose access to internal methods like itemToString it will become much harder to do properly. At the moment things seem to be in transition (e.g. some of the @internal methods are still public in the 3.0.x branch, but I assume I can't rely on that to remain the same). Before I do deep analysis to figure out what I can achieve, it would be helpful to know where this is all going to land.

Maybe for now the best answer is just to stick with 2.28.0 until 3.0 is officially released, and then revisit -- but I anticipate this will be a lot of work, and I want to better understand what is coming so I can budget time for it. First question: is there any chance of the final annotation being reconsidered, or do I definitely need to plan on major rewrites?

Regardless of the answer, please accept my thanks for maintaining this library and keeping it alive. It has served me well for a long time; I just need to figure out the best way to continue using it.

Previous behavior

I could subclass the Head* helpers (e.g. HeadLink)

Current behavior

I can no longer subclass the helpers without triggering static analysis warnings

How to reproduce

1.) Clone https://github.com/vufind-org/vufind 2.) Edit composer.json to raise the laminas-view version and run composer update 3.) Run vendor/bin/phing phpstan-console to see static analysis failure

[demiankatz]

Also, for the record, the annotations I reference were introduced in commit 4364ebd5748b51f4a6e56254fb8cf97e271e215a.

[gsteel]

Hello @demiankatz

As the person responsible for marking the helpers soft @final, I can tell you my goals for v3 but this doesn't necessarily mean all of my goals will pass the scrutiny of the TSC.

• No hidden dependencies For example, there should be no need to use setter injection for the renderer (setView), so that we can retrieve plugins. At the moment, the code base is littered with $this->getView()->plugin('someAlias'). Required collaborators should be injected into the constructor, up-front by a factory. This reduces complexity, volume of tests and introduces much improved type safety and static analysis.
• Reduce the buildup of state Lots of view helpers build up state. HeadLink is a good example of this. Sometimes state is completely unavoidable, but in view, it is frequently uneccessary. Where it is necessary, it would be useful to know which plugins are stateful so that state can be effectively managed - this gets really important if you using a runtime like Swoole or RoadRunner et al.

• Shrink public API surface and reduce maintenance burden Many classes contain endless setters and getters for option values. A lot of the getters are likely only useful for verifying setters in unit tests. There's a huge amount of 'noise' in the public apis of most of the plugins. In order for HeadLink to do its job, its role is pretty straightforward:

  • Prepend a link to the list (If not already present)
  • Append a link to the list
  • Remove a link from the list
  • Render to a string.

  I would like to remove setters/getters unless they have a real use-case, finalise all classes to force users to use composition (and reduce the BC surface area).

• De-couple from MVC Ideally, I'd like to see view shed some of its dependencies and affordances particularly to MVC. There is an existing plan to move MVC specific code to a satellite package that bridges the gap between MVC and View

I haven't looked in detail at your existing use-case in VuFind, but I would personally approach runtime evaluation of a stylesheet with something like

final readonly class ThemeStylesheet
{
  public function __construct(
    private HeadLink $headLink,
    private AssetMapper $assetMapper,
    private ThemeSetup $currentTheme,
  ) {
  }

  public function __invoke(string $assetName, ThemeSetup|null $maybeTheme = null): void
  {
    $path = $this->assetMapper->resolvePathForAsset(
        $assetName,
        $maybeTheme ?? $this->currentTheme,
    );

    $this->headLink->append($path, ['whatever' => 'attributes']);
  }
}

and then in templates, I'd be calling $this->themeStylesheet('whatever').

I see the benefit here that you can easily find anywhere in your codebase, usage of that specific plugin as opposed to general usage of the HeadLink plugin, a plugin that is much easier to test in isolation and only uses the obvious public API from 3rd party code.

I'm sure there's absolutely loads of other considerations in your project that I have no clue about so it's not presented as the right or correct way to do things, just how I'd approach a possibly similar problem…

First question: is there any chance of the final annotation being reconsidered

From my perspective, I think that finalising everything where possible is the right course of action. Other TSC members might not agree with me.

I have been working on a PoC that represents a lot of my long-term goals for laminas-view. It might be useful reference, and I'm pretty sure that no-one has looked at it apart from me 😂. Much of it is very similar to laminas-view.

[Ocramius]

@demiankatz btw, you can decide to suppress individual occurrences for now, but in the next major, those markers will move from the docblock comments to the respective class declarations, so it's messaging to allow you to think about the design :+1:

[froschdesign]

@gsteel

I have been working on a PoC that represents a lot of my long-term goals for laminas-view. It might be useful reference, and I'm pretty sure that no-one has looked at it apart from me 😂.

I looked at it and it is exactly the right direction. 👍🏻

[demiankatz]

Thanks, @gsteel and @Ocramius -- I appreciate the thoughtful preview of what is to come, and the fact that I have some advance warning to figure out all the details. This all makes sense to me, and I agree that my project probably just needs to rethink its strategy for managing assets rather than come up with a new way to make it invisible through helper subclasses. I'll open a JIRA ticket on my end to track this need, and you can of course feel free to close this issue if you wish. I'm certainly not asking you to change direction, so I don't think any action is needed on your end -- I just thought it was worth asking this question in a public place in case others are in similar situations and can benefit from the answers.

[Ocramius]

Closing then :)