Change 619076 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/core@master] POC: Skin should not be able to modify OutputPage directly

https://gerrit.wikimedia.org/r/619076

As I see it, during the Mustache skin refactoring there was simply a gap in understanding about how some of all this complex mess works. Or perhaps it was just forgotten during that one commit. That's fine and imho well within the scope the Mustache work. Its good to run into those, centralise the understanding and logic, formalise/reevaluate as needed. This particular aspect is now understood, fixed, and documented as part of change 619035.

Both of the two tasks seem to be legitimate use cases where HTML is inserted with the requirement and understanding that a certain set of styles or scripts will also be queued onto the current page. I'm not sure why we would want to disallow that. Is that the intent?

Perhaps we make this work in a better way? If so, I'd recommend looking into that first, and only later discourage the then-antipattern by social convention, or CI sniff rule, or (last resort) hard runtime overhead.

Currently, enforcing this would (and already has) led to code quality regressions where extensions try to reproduce the same set of circumstances from an unrelated different hook. This imho makes for code that is far more difficult to reason about, debug, make performant, or keep working correctly in a stable and predictable manner, and might contradict the direction proposed in T212482.

Note that there is prior art in several extensions (I think Flow, Parsoid and VE?) to formalise the set of html, modules, modulestyles, jsconfigvars in some way as a subset to be used by ParserOutput and OutputPage and other places that need to transmit these kinds of chunks.

Both of the two tasks seem to be legitimate use cases where HTML is inserted with the requirement and understanding that a certain set of styles or scripts will also be queued onto the current page. I'm not sure why we would want to disallow that. Is that the intent?

The hook being used here was called PersonalUrls - https://www.mediawiki.org/wiki/Manual:Hooks/PersonalUrls - the expectation is that hook modifies the data structure of the sidebar. I definitely don't want to have to manage the overhead of that also being allowed to do other things outside the context of the sidebar.

As a skin developer I don't want to have to keep a mental model in my head about /when/ things can be modified. If I call a method addSidebarOption I don't expect such a method to have consequences on JavaScript loaded in the page. One of the reasons I've been pushing for deprecating lots of skin hooks is I want to minimize this overhead and make our rendering dumber - there's a stage of setting up the skin's head and tail (e.g.JS+CSS), fetching the data (Skin::getTemplateData) and then a rendering stage (Skin::outputPage).

I think if the hook was called ModifySkin those consequences would have been more expected, but it's not, so yes, the intent is to moving away from the existing behaviour and make it less of a mess :). Having a single hook that modifies skin via setters may be preferable to having lots of hooks that do multiple things.

We've run into quite a few regressions during desktop refresh and most of them have related to unexpected consequences in hooks.

Perhaps we make this work in a better way?

Definitely open to ideas on this. The purpose of this task was to start those conversations. I think at the crux of this issue is that from my perspective, Skin should not need to access OutputPage, the data flow should be OutputPage to Skin.

Maybe focusing on deprecating Skin->getOutput might be a better approach here.

Change 619076 abandoned by Jdlrobson:
[mediawiki/core@master] POC: Skin should not be able to modify OutputPage directly

Reason:
Not a priority of mine right now but definitely a direction I'm interested in, in the interest of making skins more easier to maintain. I'm just not sure how to proceed and who to talk to.

https://gerrit.wikimedia.org/r/619076