TypeScript types

[micheleangioni]

The TypeScript declaration package is missing in https://www.npmjs.com/~types .

More information on this: https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html

This is very important to ensure a proper diffusion of the cloudevents standards.

[fabiojose]

@micheleangioni Could you make a PR with this? If you can't, please write an example using some piece of code from sdk.

[micheleangioni]

I do not have the complete library definitions. I have the definitions of the Spec01, Spec02 and Cloudevent classes.

These are the definitions I wrote:

declare module 'cloudevents-sdk' {
  export type SpecPayload = {
    [key: string]: any,
  };

  export class Spec01 {
    public payload: SpecPayload & {
      cloudEventsVersion: '0.1',
      eventID: string,
    };

    public check(): void;

    public type(type: string): Spec01;
    public getType(): string|undefined;

    public getSpecversion(): string;

    public eventTypeVersion(eventTypeVersion: string): Spec01;
    public getEventTypeVersion(): string|undefined;

    public source(source: string): Spec01;
    public getSource(): string|undefined;

    public id(id: string): Spec01;
    public getId(): string|undefined;

    public schemaurl(schemaurl: string): Spec01;
    public getSchemaurl(): string|undefined;

    public contenttype(contenttype: string): Spec01;
    public getContenttype(): string|undefined;

    public data(data: any): Spec01;
    public getData(): any;

    public time(time: Date): Spec01;
    public getTime(): string|undefined;

    public addExtension(key: string, value: any): Spec01;
  }

  export class Spec02 {
    public payload: SpecPayload & {
      specversion: '0.2',
      id: string,
    };

    public check(): void;

    public type(type: string): Spec02;
    public getType(): string|undefined;

    public getSpecversion(): string;

    public eventTypeVersion(eventTypeVersion: string): Spec02;
    public getEventTypeVersion(): string|undefined;

    public source(source: string): Spec02;
    // public getSource(): string|undefined; // TODO Missing getter, to be added. See https://github.com/cloudevents/sdk-javascript/pull/8

    public id(id: string): Spec02;
    public getId(): string|undefined;

    public time(time: Date): Spec02;
    public getTime(): string|undefined;

    public schemaurl(schemaurl: string): Spec02;
    public getSchemaurl(): string|undefined;

    public contenttype(contenttype: string): Spec02;
    public getContenttype(): string|undefined;

    public data(data: any): Spec02;
    public getData(): any;

    public addExtension(key: string, value: any): Spec02;
  }

  export type Extensions = {
    [key: string]: any,
  };

  export class JSONFormatter01 {
    public format(payload: SpecPayload): SpecPayload;
    public toString(payload: SpecPayload): string;
  }

  export default class Cloudevent {
    public static specs: {
      0.1: Spec01,
      0.2: Spec02,
    };

    public static formats: {
      'json': JSONFormatter01,
      'json0.1': JSONFormatter01,
    };

    public static bindings: {
      'http-structured': any,
      'http-structured0.1': any,
      'http-structured0.2': any,
      'http-binary0.1': any,
      'http-binary0.2': any,
    };

    public constructor(spec?: Spec01|Spec02, formatter?: JSONFormatter01);

    public format(): SpecPayload;
    public toString(): string;

    public type(type: string): Cloudevent;
    public getType(): string|undefined;

    public getSpecversion(): string;

    public source(type: string): Cloudevent;
    public getSource(): string|undefined;

    public id(id: string|number): Cloudevent;
    public getId(): string|undefined;

    public time(type: Date): Cloudevent;
    public getTime(): string|undefined;

    public schemaurl(schemaurl: string): Cloudevent;
    public getSchemaurl(): string|undefined;

    public contenttype(contenttype: string): Cloudevent;
    public getContenttype(): string|undefined;

    public data(data: any): Cloudevent;
    public getData(): any;

    public addExtension(key: string, value: any): Cloudevent;
    public getExtensions(): Extensions;
  }
}

[fabiojose]

@micheleangioni Now I understood. Do you want to join us and work in this issue?

[micheleangioni]

Yes, sure. I hope to find some time to write the declarations also of the remaining classes, hopefully in a couple of weeks :)

[cwallsfdc]

@micheleangioni, would be awesome!

Novice question, how to use the type definition above w/ cloudevents-sdk? I placed the type definition here lib/@types/cloudevents-sdk/index.d.ts, but it's unclear how to create a Cloudevent with a JSON string.

Thanks.

[cwallsfdc]

@micheleangioni, I had to change the typing to the following to match cloudevents.js's single export of Cloudevent.

https://gist.github.com/cwallsfdc/183c2df8dad88964efa957217b12c496

[fabiojose]

@cwallsfdc Can you help us in how to publish this typing in NPM?

[cwallsfdc]

Honestly, I think a better approach is to convert to Typescript and include the typings in the cloudevents-sdk npm module. This way the lib and typing stay aligned. Migrating to Typescript shouldn't disrupt users.

If in agreement, I can post a PR w/ Typescript changes. We'd then publish a new cloudevents-sdk version w/ typings.

Thoughts?

[fabiojose]

LGTM!

@cwallsfdc, go ahead! Perform your PR!

[grant]

Google Cloud Functions uses it's own TypeScript types because this SDK does not have them.

Google Cloud Functions Framework: https://github.com/GoogleCloudPlatform/functions-framework-nodejs/blob/v1.0.0/src/invoker.ts#L63

[grant]

Answering questions from email:

When the sdk converted to ts, we can use it in pure js projects?

Yes. When we publish to npm, we publish pure js and we have files for TypeScript types for autocompletion.

I am comfortable with this approach, using definitely typed.

We can publish this npm module instead of using DefinitelyTyped. See https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html

---

Just let me know when master and dev are the same so I can do the conversion without new changes in the middle. It will take a bit of time.

[fabiojose]

@grant Thanks for the answers.

Because of the changes that are coming from the spec, I think that is more prudence to publish the types using Definitely Typed. This will help the projects written in TypeScript and we save time to model the stuff for the future of the sdk (writing in ts) and to work in the incoming spec changes.

It's fair if we make voting:

• Vote a if you are in favor of to create just the types and publish to DefinitelyTyped
• Vote b if you are in favor of to migrate the entire codebase from JavaScript to TypeScript

What do you thing about that @grant ?

rigth now, master and develop remains the same

[grant]

b is so much better and easier. This is what Google does with all JavaScript libraries.

[fabiojose]

@micheleangioni @cwallsfdc

[cwallsfdc]

Thanks for leading this charge @fabiojose.

I agree with @grant. b. TypeScript is natural and well supported.

[fabiojose]

b wins!

Vote b if you are in favor of to migrate the entire codebase from JavaScript to TypeScript

@grant Thanks for your contribution! Let's code?

[fabiojose]

@grant

Will you code for this issue? Just to know.

[grant]

I can't continue since this repo doesn't seem to follow the CNCF standards of having reviews. Need to check on Slack and with others first.

[fabiojose]

Nice!

This is a great opportunity. You can become a new commiter and helps us to review. At this moment I am the only one: maintainer and commiter.

Because of that, I guess, you got that impression of loose standards.

Come, join us! Make this better!

[duglin]

this repo doesn't seem to follow the CNCF standards of having reviews

@grant can you elaborate? As @fabiojose mentioned, we do try to follow "normal" review processes (see the golang repo, it has more people involved) but if there's only one person active in the repo then it's hard to require multiple people for reviews :-)

[grant]

In order for me to contribute, I need master to not change as fast as it is. The commits made to master are not PRs, they are direct to master.

I would like to convert this repo to typescript, so that I can use the SDK with Cloud Functions, but last time I tried, there were commits to master that conflicted with my conversion.

All commits in this repo should be tied to a PR and all PRs are squashed so we can see the history change.

Aside, I don't really understand why we should have exceptions for reviews with certain import repos.

[takethefake]

I would like to join migrating cloudevents to typescript!

[fabiojose]

In order for me to contribute, I need master to not change as fast as it is. The commits made to master are not PRs, they are direct to master.

I would like to convert this repo to typescript, so that I can use the SDK with Cloud Functions, but last time I tried, there were commits to master that conflicted with my conversion.

All commits in this repo should be tied to a PR and all PRs are squashed so we can see the history change.

Aside, I don't really understand why we should have exceptions for reviews with certain import repos.

@grant, in the early days I used gitflow to merge to master, without PRs. Never commit directly.

Now we are using PRs, but I must approve by myself. You can help us in the review of that PRs.

Is that makes senses to you?

[grant]

Now we are using PRs, but I must approve by myself. You can help us in the review of that PRs. Is that makes senses to you?

Makes sense. I can look into converting this package again. It needs a lot of major changes though.

[fabiojose]

Folks, it's not the best, but now we have types for version 1.0.0

[Gerrit-K]

A remark from a (rather inexperienced) typescript user: the provided types are not really flexible as they are more of an interface than a type definition. I think the whole structure would be much more versatile (i.e. usable in / connectable to other libraries) if you split the event builder pattern from the bare data model and use basic object attributes instead of getters and setters in the type definition.

In my case, I wanted to receive the events from rabbitmq and store them in mongodb but both libraries expect type definitions with bare objects (and for instance iterate over their keys).

[grant]

Nice!

This is a great opportunity. You can become a new commiter and helps us to review. At this moment I am the only one: maintainer and commiter.

Because of that, I guess, you got that impression of loose standards.

Come, join us! Make this better!

Hi @fabiojose, Do you mind making me a collaborator to this repo? I have an expertise in JavaScript/TypeScript and would really like to make this project better.

At Google, we don't use this CloudEvent SDK (even though we use CloudEvents with Node) because of some of the issues described above. I would like to improve this situation but I need to be able to contribute to this repo and use best practices.

To restart trying with this repo, I've created a simple PR: https://github.com/cloudevents/sdk-javascript/pull/63

After that, I would love to help with doing things like simplifying the multiple versions we have in this repo and conversion to modern TypeScript.

[fabiojose]

For me, it's ok.

Please, file an issue requesting to be a collaborator and I will ask someone to add you.

[grant]

For me, it's ok.

Please, file an issue requesting to be a collaborator and I will ask someone to add you.

Ok, I filed an issue: https://github.com/cloudevents/sdk-javascript/issues/64

[lance]

@grant @fabiojose I would like to request that this repository NOT be converted to TypeScript. It is a mistake, in my opinion to assume that JavaScript and TypeScript are interchangeable.

[grant]

@grant @fabiojose I would like to request that this repository NOT be converted to TypeScript. It is a mistake, in my opinion to assume that JavaScript and TypeScript are interchangeable.

@lance Can you please describe more? The library will still be compatible with JavaScript. I strongly recommend using TypeScript. What specific concerns do you have?

[fabiojose]

@grant @fabiojose I would like to request that this repository NOT be converted to TypeScript. It is a mistake, in my opinion to assume that JavaScript and TypeScript are interchangeable.

@lance there is a channel at slack. Join us to discuss this issue. More heads, more thoughts

cloudeventssdk

[lance]

@grant I understand that the library can be used with Node.js if it's TypeScript and I'm fine with adding types to this module. But I personally do not use TypeScript and don't particularly want to completely change my projects to use TypeScript just to parse CloudEvents, whether or not you strongly recommend that I do. I also will stop contributing to this project if it's converted to TypeScript and instead write another Node.js module. Of course, my contributions can be replaced - and there won't be any hard feelings - but over the last month I am by far the most prolific contributor here.

¯_(ツ)_/¯

[lance]

@fabiojose I've sent a request to join the Slack

[helio-frota]

My TS knowledge still limited to start/continue contributing

[grant]

Hi @lance, I think there may be some confusion, which is completely understandable.

You don't need to change any of your projects to use TypeScript. We don't expect anybody to do that. I assume many projects will just use vanilla Node now and in the future.

I understand as a personal (and large) contributor to this project who might not be using TypeScript, it might seem like we are coming from left-field asking for this. I thought the same before I used TypeScript too.

JavaScript is valid TypeScript. It's a strict superset. The first step would be very simple:

• Change .js files to .ts
• Add a watch/build step (npm run build)
• Develop in Node as normal

All contributors can use Node as they have been used to, but need to run the watch command. There may be some cases where we will want to add types or interfaces, but I don't think that will be an issue.

[lance]

Hi @grant - just to be clear, I have used TypeScript and understand that it's valid JavaScript.

However

• Change .js files to .ts
• Add a watch/build step (npm run build)

This is exactly what I object to. That and the verbose non-intuitive syntax of TypeScript. And the added code size.

With good linting, proper use of class, private fields, and complete test coverage (all things any modern JavaScript project should have) the benefits of TypeScript just don't seem very persuasive to me.

Respectfully, I understand that you may like TypeScript more than vanilla JavaScript. But I don't accept that it's a de facto improvement and I have not seen any real justification for completely rewriting this module. I will be right there with you in saying that the module needs improvement. But this is not the path I want to take in order to do that.

[lance]

Just wanted to add... I realize that in some circles this is not a popular opinion. I don't mean to be abrasive about it. I'm just a purist and do not believe TypeScript is by default an improvement.

[grant]

Can you be more specific as to what you object? I said we don't have to use any special TS syntax to start. The code size would not be different, there is no impact on user-executed code. Do you not want to have a different file extension? Do you not want to run the watch command?

If I told you that Google, Microsoft, Lyft, Slack, Asana, and others use TypeScript because of clear benefits with tooling, testability, refactorability, navigation, would that make a difference?

In the above comment, we're threatening to stop contributing to this project and saying that we're the most prolific contributor here. That isn't what the CloudEvent format and CNCF community is about. We're creating an open-source community tool.

Are there specific incompatibilities at Red Hat that this would cause? Is it just personal opinion? If a majority of folks wanted TS support, would you allow it?

[lance]

Let's be clear - I can't allow or disallow anything. I'm just a community contributor who has become invested in this project. I don't have write permission on this repository or any level of contributor status other than as a community member. I haven't even submitted an issue asking to be added as a collaborator.

I apologize for my apparent self-aggrandizement as the most prolific contributor in the last month. It had seemed to me that the project was not being very well maintained, so I jumped in and started contributing code. And while my contributions have been slow to land, I feel invested in the project and would like to make it successful. I too would like to create a widely used and high quality open source community tool. I'm not unfamiliar with the paradigm.

It was not meant as a threat that I would stop contributing. It's just the way it is. I don't have to use this module. I would like to, but if it changes to TypeScript I will likely move away from it. I'm sorry if it came across as threatening.

I know that lots of big companies use TypeScript. Lots of big companies use COBOL. This argument doesn't persuade me.

Regarding Red Hat... I'm not speaking for the company, though I am using this module in support of our objectives. I know that among the peers in my immediate circle at Red Hat, most prefer not to use TypeScript. I don't know what you mean by "a majority of folks". It's not clear that many people are using this module. So if there are 4 people in favor of TypeScript and myself against it, I guess I'm out numbered.

Now, just in case I misunderstand what you are saying, I should be clear. I have no objection to declaring and publishing types such as this: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/opossum/index.d.ts#L5. What I object to is writing my code like this: https://github.com/GoogleCloudPlatform/functions-framework-nodejs/blob/master/src/invoker.ts.

[grant]

OK, thanks for the response. And thanks for your contributions.

I hope we don't have unreadable code. I hope we have a more usable module with active developers like yourself that can improve the CloudEvent ecosystem.

There are a few logistics/issues we need to solve before adding TypeScript type support anyhow. I'm curious as to how you're using this module currently, and perhaps how it could be improved. I'll be keeping the usage of this module in https://github.com/boson-project/faas-js-runtime in mind.

[lance]

@grant good deal. I don’t want unreadable code either. I want this to be high quality idiomatic JavaScript and of course, with Typescript type support. :) IMO one good improvement might be this: https://github.com/cloudevents/sdk-javascript/issues/65

[erikerikson]

@lance I've been only recently picking up typescript so please excuse my ignorance. Is there an automated process in opossum (or an example elsewhere) that validates that as changes occur that the type declarations do not drift or more pointedly that the typescript consumers aren't caught out?

[grant]

@lance I've been only recently picking up typescript so please excuse my ignorance. Is there an automated process in opossum (or an example elsewhere) that validates that as changes occur that the type declarations do not drift or more pointedly that the typescript consumers aren't caught out?

The problem with separate type declarations is that they are not updated with the source code.

This is what was done in the repo before, but they were not updated with the source. Thus, the types were inaccurate and unusable. See https://github.com/cloudevents/sdk-javascript/issues/9#issuecomment-550335296

As this repo changes too much, the only option would be to use TypeScript to generate types.

[erikerikson]

Thanks @grant I hear that concern. If the automated validation means I am asking about was in place, I wouldn't expect type drift to have occurred.

[lance]

@grant there is not anything automated for opossum type generation. I agree this is not ideal. The types for that module are mostly maintained by external contributors who have an interest in using TS.

I just skimmed this article, but it looks promising. https://dev.to/open-wc/generating-typescript-definition-files-from-javascript-5bp2

They are auto generating types from JavaScript functions which contain valid jsdoc (which we of course need anyway).

[lance]

FWIW - I would like ultimately for the user-facing API to be a little more stable and expose far less. Ideally, we have this:

const { CloudEvent, HTTPReceiver, HTTPEmitter } = require('cloudevents-sdk');

There's not much more that a developer needs to be concerned with IMO. All of the parsers and unmarshallers and all of that are internal to the module and don't need to be exposed. Keeping the API footprint small and clean should also help quite a bit.

[erikerikson]

Hi @lance - pretty sure it was me that asked about automation ;D Would you be willing to add the automation for validating that drift doesn't happen? It appears that the relevant PR called out by that article has landed (it targeted 3.7 but 3.8 is out and 3.9 is RC). Is it working? If so is this really going to provide the same level of type validation or does it move responsibility off of the compiler to the maintainers and reviewers?

@grant it's not clear to me that js-docs are less arduous than typescript but if this solution generated the necessary *.d.ts files and validation of type declaration and safety were part of an automated CI process run against all PRs in this repo, would it satisfy your needs if not preferences?

[grant]

Unfortunately JSDoc comments don't satisfy deep accurate autocompletion or other benefits of TypeScript such as optional static typing. Besides, code comments aren't enforced or always complete/accurate. Generating .d.ts files doesn't sound great even if it could be done.

Converting to ES6 gets some autocompletion, but not as thorough or deep as TypeScript. The IDE experience is awesome. We could dramatically reduce tests while ensuring increased safety. Example in TS:

Again, we don't need to change anything besides renaming .js files to .ts to get this benefit.

[lance]

@erikerikson sorry for the misattribution! :) @grant JSDoc can be enforced with eslint. We already have a very basic linting rule that enforces valid JSDoc if JSDoc exists. This can be changed to require JSDoc. I think we'd want to clarify exactly what API should be exposed and only enforce it on those files. But I think it might not be as difficult as you imagine to enforce this if we keep the surface area of the API much smaller than it is today.