ResourceLoaderContext was added on later but has since become a critical part of how modules are composed. The context has to be passed around to virtually every method, and most members have to be two or three dimensional arrays fragmented at the top by a hash of the request context. There seems to be very little use for this as the construction phase is cheap, and it's extremely unlikely we'd have to instantiate the same module for different contexts in the same http request (other than unit tests).
Description
Description
Event Timeline
Comment Actions
Modifying the ResourceLoaderModule constructor is going to be a breaking change, and will require updating a bunch of extension's constructors to call the parent.
Another option might be to add a setContext() and getContext() methods so the context is set after the object is constructed. getContext would use ResourceLoaderContext::newDummyContext() if setContext was never called.
Comment Actions
Expanding on the setContext() idea: use setContext() after constructing a ResourceLoaderModule object, and, when a context is needed in a function, use the context set with setContext(); if there is a context provided as a parameter, use that context instead, and if no context is provided and no context has been set, then use a newDummyContext(). That way the change should not break anything and can be used where the context remains the same or where the context changes.
Comment Actions
I think there's more to it.
Having some context available through some method is not the objective here. The goal is to not require fragmentation within a module object for every possible context that might be passed in. And instead require the caller to construct a new module object if they need a different context.
Whether we change the constructor signature or not doesn't change the fact that the intention here is to have a module object only used for 1 context. Thus we need to require callers to instantiate the module class themselves if they need a different context.
Adding this method in itself isn't going to improve anything. Before starting work, let's sketch out here on Phabricator what our plan is.
- For example, getVersionHash() uses a derivative context for its method calls.
- If we add setContext(), which code path has the authority to set the context for that module object? Right now the authority is in ResourceLoader::getModule(). Should we change that method to fragment its cache by context hash?
- Eventually, at the end of all this, we need to be able to ignore different ResourceLoaderContext objects passed as parameter to module methods, and able to safely remove the in-object cache fragmentation by context hash. Probably by removing them from the module method signatures (which also requires extension updates, so perhaps adding to constructor isn't so bad?)