feat: application service types default to any

[vonagam]

In #1364 ServiceTypes default changed from any to {} with an intention to make app.service() act more strict when ServiceTypes is provided. Desired outcome and expected type resolution (specified in second comment from that PR) were right, but the solution had a flaw.

When you make ServiceTypes default to {}, Application without parameters (like in HookContext) becomes Application< {} >. And you can't assign Application< {} > to Application< MyServiceTypes >:

const app: Application< MyServiceTypes > = context.app; // error
const app: Application< MyServiceTypes > = context.app as Application< any >; // works

As shown Application< any > does not have such drawback.

To achieve desired outcome right solution is to simply remove second catch-all service() overload, because when ServiceTypes is any first definition is already catch-all and when ServiceTypes is not any then first definition acts as a strict one.

There is a little breaking change (that why it is not a fix, but a feat) since now {} stops acting as wildcard and not a default value.

[deskoh]

This will enforce compulsory typings for all service.
Currently. if ServiceTypes is not defined, all service will default to any.

Possible to have best of both worlds?

[vonagam]

This will not enforce compulsory typings.

Here is the code which you can check in typescript playground:

interface ApplicationOld< ServiceTypes = {} > {
  service< L extends keyof ServiceTypes >( location: L ): ServiceTypes[ L ];
  service( location: string ): keyof ServiceTypes extends never ? any : never;
}

interface ApplicationNew< ServiceTypes = any > {
  service< L extends keyof ServiceTypes >( location: L ): ServiceTypes[ L ];
}

const oldUnspecified = undefined as any as ApplicationOld;
const oldSpecified = undefined as any as ApplicationOld< { foo: 'bar' } >;
const newUnspecified = undefined as any as ApplicationNew;
const newSpecified = undefined as any as ApplicationNew< { foo: 'bar' } >;

const oldUnFoo = oldUnspecified.service('foo');
const oldUnBaz = oldUnspecified.service('baz');
const oldSeFoo = oldSpecified.service('foo');
const oldSeBaz = oldSpecified.service('baz'); // never
const oldConvert: ApplicationOld<{ foo: 'bar' }> = oldUnspecified; // error;

const newUnFoo = newUnspecified.service('foo');
const newUnBaz = newUnspecified.service('baz');
const newSeFoo = newSpecified.service('foo');
const newSeBaz = newSpecified.service('baz'); // type cheching error
const newConvert: ApplicationNew<{ foo: 'bar' }> = oldUnspecified;

[deskoh]

Ah. I get it now.

[vonagam]

Was this closed accidentally too?

[daffl]

Dang, yes, sorry. This should also stay open to be either merged or revisited for v5.

[vonagam]

Rebased and adapted.

[daffl]

Great, thank you! Will have a look shortly but if it doesn't break any of the latest tests this should work well.

[daffl]

Perfect! This totally should fix some of the problems I was having before, too. Will go out with the beta release shortly.

[daffl]

Hm, strange, looked like the build passed but is failing once it was merged.

[vonagam]

Any particular problem?

[daffl]

Type incompatibility: https://github.com/feathersjs/feathers/runs/2156522850?check_suite_focus=true#step:6:235

Getting the errors locally with the latest dove branch, too 🤔

[daffl]

I have a feeling this might be something else. The compilation error I'm getting locally has something to do with the grant package.

[daffl]

Other problem fixed for now and I am getting the same error:

Error: test/index.test.ts(27,11): error TS2322: Type 'import("/home/runner/work/feathers/feathers/packages/express/src/declarations").Application<any, any>' is not assignable to type 'import("/home/runner/work/feathers/feathers/packages/feathers/lib/declarations").Application<any, any>'.
  The types returned by 'service(...)' are incompatible between these types.
    Type 'import("/home/runner/work/feathers/feathers/packages/feathers/lib/declarations").FeathersService<import("/home/runner/work/feathers/feathers/packages/express/src/declarations").Application<any, any>, import("/home/runner/work/feathers/feathers/packages/feathers/lib/declarations").Service<any, Partial<any>>>' is not assignable to type 'import("/home/runner/work/feathers/feathers/packages/feathers/lib/declarations").FeathersService<import("/home/runner/work/feathers/feathers/packages/feathers/lib/declarations").Application<any, any>, import("/home/runner/work/feathers/feathers/packages/feathers/lib/declarations").Service<any, Partial<any>>>'.
      Type 'FeathersService<Application<any, any>, Service<any, Partial<any>>>' is not assignable to type 'ServiceAddons<Application<any, any>, Service<any, Partial<any>>>'.
        Types of property 'hooks' are incompatible.
          Type '(options: HookOptions<Application<any, any>, Service<any, Partial<any>>>) => 

Problem seems to be that the Express app type is no longer compatible with the basic Feathers app type (apparently it was before for some reason).

I tried updating express/src/declarations.ts with the following:

interface ExpressUseHandler<T, ServiceTypes> {
  <L extends keyof ServiceTypes & string> (
    path: L,
    ...middlewareOrService: (
      Express|express.RequestHandler|
      (keyof any extends keyof ServiceTypes ? ServiceInterface<any> : ServiceTypes[L])
    )[]
  ): T;
  (...expressHandlers: express.RequestHandler[]): T;
  (handler: Express|express.ErrorRequestHandler): T;
}

export interface Application<ServiceTypes = any, AppSettings = any>
  extends FeathersApplication<ServiceTypes, AppSettings>, Omit<Express, 'listen'|(keyof FeathersApplication)> {
    listen(port: number, hostname: string, backlog: number, callback?: () => void): Promise<http.Server>;
    listen(port: number, hostname: string, callback?: () => void): Promise<http.Server>;
    listen(port: number|string|any, callback?: () => void): Promise<http.Server>;
    listen(callback?: () => void): Promise<http.Server>;
    use: ExpressUseHandler<this, ServiceTypes>;
  }

But no luck so far.