lib: rewrite in TypeScript #226
Conversation
lance
added
type/enhancement
lance
requested review from
danbev,
grant and
fabiojose
and removed request for
a team
|
I noticed a |
src/transport/protocols.ts
Outdated
| @@ -0,0 +1 @@ | |||
| export const enum Protocol { HTTPBinary, HTTPStructured, HTTP }; | |||
There was a problem hiding this comment.
For small files like these, it might be worth it to inline it rather than have a new file.
For example, we can inline the contents into the index.ts file.
|
@lholmquist good catch! That's actually a bug on master. 🐛 |
This is a major rewrite of the entire codebase into TypeScript. Nearly all tests have been retained except where behavior is significantly different. Some highlights of these changes: * lowercase all CloudEvent properties and fix base64 encoded data Previously there was a format() function that would convert a CloudEvent object into JSON with all of the properties lowercased. With this rewrite a CloudEvent object can be converted to JSON simply with JSON.stringify(). However, in order to be compliant with the JSON representation outlined in the spec here https://github.com/cloudevents/spec/blob/v1.0/json-format.md all of the event properties must be all lowercase. * lib(transport): make transport mode an Enum * src: allow custom headers (#1) * lib(exports): export explicitly versioned names where appropriate * lib(cloudevent): modify ctor to accept extensions inline * lib(cloudevent): make extensions a part of the event object * test: convert all tests to typescript * examples: update all examples with latest API changes * docs: update README with latest API changes * src: add prettier for code style and fix a lot of linting errors * lib: move data decoding to occur within the CloudEvent object Signed-off-by: Lance Ball <[email protected]>
|
@cloudevents/sdk-javascript-maintainers I have completed all of the items on the checklist and addressed about 95% of the comments/nits. Please have a look. This is ready for final review and landing. |
Signed-off-by: Lance Ball <[email protected]>
Signed-off-by: Lance Ball <[email protected]>
| const properties = { ...event }; | ||
|
|
||
| this.id = (properties.id as string) || uuidv4(); | ||
| delete properties.id; |
There was a problem hiding this comment.
These deletions seem unnecessary and do not affect the logic of the function. Do we really need to delete them? The whole properties const is only scoped for this block and will be deleted at the end of the block naturally.
There was a problem hiding this comment.
As noted in the comment above, it is necessary to delete them if we want to have extensions as siblings of the event properties. We remove all of the known properties, and what's left are the extensions.
|
Looks good! 👍 A few comments. |
Signed-off-by: Lance Ball <[email protected]>
Signed-off-by: Lance Ball <[email protected]>
|
Deep breaths.... landing this. |
This is a major rewrite of the entire codebase into TypeScript. Where the code was overly complex, I have attempted to simplify things. A few things remain before it can land.
/examplesto work with the new APICloudEvent#extensionsand how they are mapped to https://github.com/cloudevents/spec/blob/v1.0/json-format.mdSome notes about the changes...
./distlibfolder has been removed and everything under it was pulled up intosrclib/bindingsfolder has been moved tosrc/transportFixes: #223
Fixes: #219
Fixes: #203
Fixes: #195
Signed-off-by: Lance Ball [email protected]