loopback-next icon indicating copy to clipboard operation
loopback-next copied to clipboard

Can't override app.start if RepositoryMixin used: TS2425

Open mgabeler-lee-6rs opened this issue 2 years ago • 3 comments

Steps to reproduce

  1. Create an app that uses RepositoryMixin
  2. Try to override the application start method

Current Behavior

error TS2425: Class '... & RestApplication' defines instance member property 'start', but extended class 'Application' defines it as instance member function.

Expected Behavior

It should compile!

It used to compile just fine in older releases.

Link to reproduction sandbox

  1. clone https://github.com/mgabeler-lee-6rs/lb4-bugs
  2. run npm run build

Additional information

Playing around with the mixins:

  • BootMixin(RestApplication) -- OK
    • BootMixin re-declares the start member
  • BootMixin(RepositoryMixin(RestApplication)) -- fails
  • RepositoryMixin(BootMixin(RestApplication)) -- fails

Looking through the history of repository.mixin.ts, the only thing I see that I could point to as a probable cause for this is https://github.com/loopbackio/loopback-next/pull/5394/files#diff-11574932d711bedee7d0693e1b5f50ebfd143a78cc62e36bb31ea5e0e711140c

Overriding application.start is "documented as OK" if you will, insofar as the hello-world example uses it almost exactly the way my repro does, the difference being that hello-world doesn't have the repos mixin: https://github.com/loopbackio/loopback-next/blob/master/examples/hello-world/src/application.ts#L23

Related Issues

Nothing found yet

mgabeler-lee-6rs avatar Nov 16 '21 22:11 mgabeler-lee-6rs

Digging in, the suspected PR/diff is almost certainly the issue: the mapped type used for MixinTarget is converting every "member" of the super class to a property. There doesn't seem to be any good way around this in TypeScript that I can find :(

One possible, though admittedly very ugly, workaround: Make RepositoryMixin do no-op overrides of start and stop so that they override this nature, e.g. insert code like this near the end of the mixin class:

    // workaround https://github.com/loopbackio/loopback-next/issues/8079
    // @ts-ignore
    public async start(): Promise<void> {
      return super.start();
    }
    // @ts-ignore
    public async stop(): Promise<void> {
      return super.stop();
    }

The need for @ts-ignore is unfortunate, but I see that BootMixin has this too, presumably for the same reason.

mgabeler-lee-6rs avatar Nov 16 '21 23:11 mgabeler-lee-6rs

I'm wondering if this bug was introduced in one of the newer TypeScript versions. Will need to look more into this. In the mean time, if you were able to find a more permanent solution, please do post them here so that we can add it to the docs.

achrinza avatar Nov 18 '21 23:11 achrinza

My workaround was just to copy the ts-ignore strategy, i.e. I prefixed my start overrides with:

	// https://github.com/loopbackio/loopback-next/issues/8079
	// eslint-disable-next-line @typescript-eslint/ban-ts-comment
	// @ts-ignore

mgabeler-lee-6rs avatar Nov 19 '21 22:11 mgabeler-lee-6rs