Combination of language negotiation and path aliasing can cause a corrupted route cache, 404s

Note that this will then require the core version that the core patch will be committed against, not sure if it will be backported to 8.4, possibly. Without a supported core version, this will be a fatal error as $this->languageManager will not exist.

Yes, I knew you were aware of this. This was an information for people who want to try the patch and the maintainer that he needs to consider that when committing this/releasing a new version (e.g. by addingadding an explicit version dependency on whatever core version that will contain the fix in core)

So for the moment this only works on 8.6.x when the core patch is also installed?

Yes, but what version(s) of core does that patch apply cleanly to?

I'd expect the patch applies to 8.4+, but the question is against which versions it will be committed in the end. It is a critical bugfix, which means it is still viable for 8.4 but since it's a constructor change, might not get in, but i really hope it will get into 8.5.

Testing notes:

* The original cache behavior is covered in DomainConfigHomepageTest.
* That needs to be extended to cover languages as well, see RouteCachingLanguageTest in the core patch.

The larger question here is if we have to move or adjust our routing behavior. At the moment, only Domain Config cares about the route cache and that was because the homepage was getting cached incorrectly.

We may need to assume that any code that relies on Domain may run into this issue, which would mean moving that service change into the main module.

See for instance, how DomainRedirectResponse is handled.

Perhaps the DomainRouteProvider needs to be in the main module and can be swapped in when other modules request?

That aside, its implementation needs to check for the presence of the LanaguageManager service and load it if needed.

I just looked at this code, and there is an easier solution for 8.5 and 8.6, but it breaks 8.4, since https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Routing%2... was just added.

And a patch that should work on 8.4, 8.5. and 8.6.

Testing at https://github.com/agentrickard/domain/pull/411

I don't think there's anything wrong with requiring 8.5 in a newer version of domain, you'd just add drupal:system (>= 8.5) in info.yml, and then users see that they should not update that module before the update to 8.5. Otherwise you have to keep your implementation in sync with whatever core is doing there, will just overriding the cache id method is less likely to get out of sync.

I have committed this (with a shim for 8.4 support), but need to fix https://www.drupal.org/project/domain/issues/2976127 before pushing to Drupal.org.

And this fix -- https://www.drupal.org/project/domain/issues/2976127 -- would drop support for 8.4, so that shim can go.

This has been committed.