Should we really cache the "HuxDiscovery" object?

This leads to possible scenarios where the class is incomplete - see the exception in the discovery() method.

The exception is thrown when implementations is empty, this property is only needed for the discovery method, it isnt needed if the object is woken up. You can see this property is set during the constructor in \Drupal\hux\HuxModuleHandler::discovery only on cold starts.

If the object is taken out of cache in \Drupal\hux\HuxModuleHandler::discovery, the discovery method is never invoked.

The exception message is perhaps misleading in that it only catches abuse of the class itself, that is the discovery() method will never in fact be called again.

The discovery class and is mechanics are all internal. Nobody but Hux relies on its particular behaviour.

We could cache either the raw data, or another object that only holds the data but that is not itself responsible for discovering it.

The intention of the discovery class is that it is both responsible for the raw data and discovery. The implementations property and discovery() method must/can not be used when awoken.

--

Asides

Its not necessary to enable cache, see the optimise option. I havn't seen any noticable slowdown in performance with profiling. (Given PHP is configured appropriately, opcache size large enough, etc)

The discovery object is put into its own named cache such that you could put it into an ultra fast (chained fast-disk/redis) backend so it does not enter a slower database-backed cache.

Closing per above

Just to clarify:
For issues like this obviously it "works as designed".

I think there are two questions:
- Would it be nicer, cleaner etc to change it as proposed here?
- Is it actually worth it to change it now? And will it help as preparation for other issues?

So, you could see it as a late "hindsight" code review, which of course reflects my personal opinion.
I thought this can be useful before we go to core.

The combined branch at 📌 Combined refactoring and attributes interface Needs review does include this change, but I thought it better to have a dedicated issue to discuss it.
https://git.drupalcode.org/project/hux/-/merge_requests/15/diffs?commit_...