[Proposal/Discussion] update Typescript definitions

[baadc0de]

Hi all,

I propose to update the Context type definition to be parametrized over <P, M> where P is the type of the params prop and M is the type of the meta prop.

Namely:

class Context<P = {}, M = {}> {
  constructor(broker: ServiceBroker, action: Action);

  params: P;
  meta: M;

  /* ... */
}

This is similar to how guys in React defined the Component interface, see: https://medium.com/flow-type/even-better-support-for-react-in-flow-25b0a3485627

I would also propose to remove this definition: https://github.com/moleculerjs/moleculer/blob/master/index.d.ts#L141 from the Service class as it effectively disables type checking on this when deriving from Service.

Additionally, I would propose to change the constructor handling of Service where it needs a schema: ServiceSchema argument, because this is not formally available to the class before a super call is finished. This means that your service schema would not be allowed to call / modify this. I would like to make this argument optional.

Instead, I propose to add to the definition the parseServiceSchema method, which can be invoked on this after the super call.

Let me know if there are any other takes on this. The goal is to make TS / Flow ES6 classes interoperate better with moleculer-runner and enable stricter type checking with less code.

[icebob]

Ping @ColonelBundy @dani8art @rmccallum81

[ryanm-pm]

I would set the default to the GenericObject type to allow for the limitation of value types to exclude functions and other types that can't be transfered from node to node.

I also agree about the parseServiceSchema method. It would make extending the Service simpler.

Removing the definition would remove compatibility with loading services from a definition then accessing this to add and read variables. I don't think this is a terrible thing as far as type safety goes. I believe the services always should be created using extended classes but that is a design decision.

[baadc0de]

I support the GenericObject idea.

Removing the definition would remove compatibility with loading services from a definition then accessing this to add and read variables. I don't think this is a terrible thing as far as type safety goes. I believe the services always should be created using extended classes but that is a design decision.

Hmm, I don't follow. Could you please post a small example of how this plays together?

[ColonelBundy]

I approve, good ideas 👍

[ryanm-pm]

@baadc0de I don't think I did a good job wording that. What I was getting at is the Service class can have methods defined by the methods property of the ServiceSchema which are not initially defined at the this level. We could get around this by making the ServiceSchema a generic so you can add an interface that defines these methods and additional properties. I'll attach a proposal for that change in a bit.

[rmccallum81]

As promised here's a possible change for making the ServiceSchema more generic to allow the methods to be properly typed.

interface ServiceSchema<S extends Service = Service> {
    name: string;
    version?: string | number;
    settings?: ServiceSettingSchema;
    dependencies?: string | GenericObject | Array<string> | Array<GenericObject>;
    metadata?: GenericObject;
    actions?: Actions<any, S>;
    mixins?: Array<ServiceSchema>;
    methods?: ServiceMethods<S>;

    events?: ServiceEvents;
    created?: () => void;
    started?: () => Bluebird<void>;
    stopped?: () => Bluebird<void>;
    [name: string]: any;
}

Obviously more needs to be done than just this change but it is a possible solution.

[icebob]

After #233, can I close it? Is it done?