Update http route registration in migration guide and examples

[rudolf]

Summary

Update http route registration in migration guide and migration examples. Make it clearer that a shim exposing server.route isn't NP compatible, but can be a helpful way to break down the migration into steps.

[skip-ci]

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

• This was checked for cross-browser compatibility, including a check against IE11
• Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
• Documentation was added for features that require explanation or tutorials
• Unit or functional tests were updated or added to match the most common scenarios
• This was checked for keyboard-only and screenreader accessibility

For maintainers

• This was checked for breaking API changes and was labeled appropriately
• This includes a feature addition or change that requires a release note and was labeled appropriately

[elasticmachine]

Pinging @elastic/kibana-platform (Team:Platform)

[jasonrhodes]

Is there any intention to provide core.http.route as a method in the NP, that works like this? If not, then I think showing this as a "shim" is confusing because I interpret "shim" as "massage the old way so that it looks and works like the new way".

[jasonrhodes]

In other words if in NP core.http exists, but core.http.route isn't going to exist, then this shim breaks your future code in a hard to notice way.

If we need an example of how to keep using legacy router in the shim (do we need this?) I would say maybe shimming it as core.legacyHttp.route is a clearer way of indicating what's going on.

[rudolf]

core.http.route will never exist.

If we need an example of how to keep using legacy router in the shim (do we need this?) I would say maybe shimming it as core.legacyHttp.route is a clearer way of indicating what's going on.

I think the intention is to use types to minimize the legacy API surface that's depended on and make these dependencies explicit. So instead of just say "I depend on Legacy.Server" you narrow it down to just the hapi router or just a dependency on another plugin exposed via the server object.

I agree that this introduces confusion in this case and like your idea of making it more explicit. The shimming instructions were written before most of the API's were available in CoreSetup. Now that the real coreSetup has many useful API's another approach might be to inject three variables into setup:

import { CoreSetup } from 'src/core/server';

export interface LegacySetup {
  server: {
    route: Legacy.Server['route']
  }
}

export class Plugin {
  public setup(core: CoreSetup, plugins: {}, legacy: LegacySetup) {
    legacy.route({...})
  }

This gives you the added safety that you're relying on Core's CoreSetup so you don't have to wonder if your shimmed MyPluginCoreSetup is accurately shimming core.

[jasonrhodes]

I like that idea a lot to be honest.

[joshdover]

Another option is to use the __legacy namespace that some plugins have been doing:

import { CoreSetup } from 'src/core/server';

export interface DemoPluginCoreSetup extends CoreSetup {
  __legacy: {
    server: {
      route: Legacy.Server['route'];
    };
  };
}

export class Plugin {
  public setup(core: CoreSetup, plugins: {}) {
    core.__legacy.server.route({...})
  }
}

[jfsiii]

I like what you have here and would be happy to see it merge.

core.http.route will never exist.

Is that correct? I opened #44174 about core.http.route and @restrry replied "should be done as a part of #44620" so I thought it was coming eventually

[rudolf]

We have broad consensus that registering routes with a configuration object could be a nicer API but I don't think we have concrete designs for how it will be exposed. I imagined that we might still have a core.http.createRouter(path: string) method like today, but that the Router instance returned from this would allow you to do router.add({...}) or router.register({...})

However, this conversation kinda changed my mind on the our approach to shimming plugins. We used to recommend constructing a CoreSetup that looks as close as possible to what CoreSetup will look like in the New Platform. This has caused a lot of confusion and speculation which I realise now doesn't add much value.

As @jasonrhodes pointed out:

then this shim breaks your future code in a hard to notice way.

Instead of making it look the same, I think the goal should rather be to have legacy be as different and noticeable as possible, and make the exact legacy API surface explicit by picking only the legacy methods that you depend on.

[jfsiii]

Exposing a function on a router instance, vs core.http is also what I was expecting. Thanks for the explanation & confirmation. Sorry for the noise.

[joshdover]

Thanks for the TOC

[joshdover]

👍