Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New type for ServiceSchema breaks ServiceBroker.createService and interface Options #1297

Closed
4 tasks done
jeffrson opened this issue Aug 12, 2024 · 7 comments · Fixed by #1299
Closed
4 tasks done

Comments

@jeffrson
Copy link

jeffrson commented Aug 12, 2024

Prerequisites

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

  • I am running the latest version
  • I checked the documentation and found no answer
  • I checked to make sure that this issue has not already been filed
  • 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
@jeffrson jeffrson changed the title New type for ServiceSchema breaks ServiceBroker.createService New type for ServiceSchema breaks ServiceBroker.createService and interface Options Aug 12, 2024
@MichaelErmer
Copy link
Contributor

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
Copy link
Member

icebob commented Aug 25, 2024

Is it related to this merged PR? #1272

@MichaelErmer
Copy link
Contributor

MichaelErmer commented Aug 30, 2024

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
Copy link

icex commented Sep 4, 2024

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
Copy link
Contributor

@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
Copy link
Author

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
Copy link
Member

icebob commented Oct 17, 2024

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

image

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants