New type for ServiceSchema breaks ServiceBroker.createService and interface Options

jeffrson commented 3 months ago

Prerequisites

Please answer the following questions for yourself before submitting an issue.

[x] I am running the latest version
[x] I checked the documentation and found no answer
[x] I checked to make sure that this issue has not already been filed
[x] I'm reporting the issue to the correct repository

Current Behavior

Due to recent changes in Typescript definition of ServiceSchema our code fails with tsc for options.started and options.stopped. AFAICT, I need to expand ServiceSchema with a "ServiceThis". When I (try) to do this (I'm unfortunately no Typescript expert), tsc fails for ServiceBroker.createService since it does not accept any other "ServiceThis" as void. There is no way to specify any other type. The same applies to interface Options (when explicitely used as type), if I'm not mistaken.

Besides, I'm really astonished how such a breaking change went into a patch release. At least, instead of void it probably would be a better choice to use Service as default type for ServiceThis.

Context

Please provide any relevant information about your setup. This is important in case the issue is not reproducible except for under certain conditions.

Moleculer version: 0.14.34

MichaelErmer commented 3 months ago

We are also experiencing issues with this change; it seems to be originating from the new generic for ServiceSchema which makes totally sense, but maybe it would be better to default T to Service<ServiceSettingSchema> instead of void.

This is what you would have to do for all services... const service: ServiceSchema<ServiceSettingSchema, Service<ServiceSettingSchema>> = ...

We decided against migrating to 0.14.34 for now.

icebob commented 3 months ago

Is it related to this merged PR? https://github.com/moleculerjs/moleculer/pull/1272

MichaelErmer commented 2 months ago

Is it related to this merged PR? #1272

yes, this is the issue: https://github.com/moleculerjs/moleculer/pull/1272/files#diff-7aa4473ede4abd9ec099e87fec67fd57afafaf39e05d493ab4533acc38547eb8R730

default should be Service<ServiceSettingSchema>

icex commented 2 months ago

yup. got the same behaviour trying to upgrade from .33 to .34. Everything breaks

error TS2339: Property 'logger' does not exist on type 'void'.

marceliwac commented 2 months ago

@jeffrson, really sorry this has caused you problems. I clearly failed to test the use-cases which didn't specify the types for ServiceSchema. The patch I just submitted should address your issues with types for lifecycle handlers.

I'm not sure which Options Interface you are referring to, could you provide some details that would make it possible for me to reproduce this behaviour?

jeffrson commented 1 month ago

I just realized that the Options interface I was referring to actually comes from another, independent, package: moleculer-decorators (https://github.com/ColonelBundy/moleculer-decorators/blob/master/src/index.ts#L29).

That package relies on v0.14 - which confirms another time why it was a really bad idea to publish the change with a semver compatible version number.

Nevertheless, the reason of the problems is the same: there is no way to "inject" an certain "ServiceThis" into createService or Options.

@marceliwac the publication in a patch release is not your fault I guess ;-)

icebob commented 1 month ago

@jeffrson I'm releasing the PRs based on the creator's choice and #1272 was flagged as non-breaking.

By the way, I've merged the #1299 but it would be good if somebody can try it and gives feedback that I can release it

moleculerjs / moleculer