Repository navigation
Use Direct Object Creation Pattern, Do Not Use Builder Pattern #65
Description
The JavaScript SDK uses a Java-like builder pattern for creating events.
This is not ideal as it is not very JavaScript-like, and is unnecessarily wordy. As we are in JavaScript, we can use direct JSON objects for creating the CloudEvent payload.
Here is a sample taken from the README:
Actual
let myevent = event()
.source('/source')
.type('type')
.dataContentType('text/plain')
.dataschema('http://d.schema.com/my.json')
.subject('cha.json')
.data('my-data')
.addExtension("my-ext", "0x600");Expected
import {CloudEvent} from 'cloudevents/v1';
let myevent = new CloudEvent({
source: '/source',
type: 'type',
dataContentType: 'text/plain',
dataschema: 'http://d.schema.com/my.json',
subject: 'cha.json',
data: 'my-data',
addExtension: ["my-ext", "0x600"]
});Reacted by Helio Frota
Activity
fabiojose commentedon Apr 27, 2020
LGTM
lance commentedon Apr 28, 2020
I agree that this would be an improvement in the API. It makes me think about versioning. This is a breakable change to the API. So, presumably it would need a major version number bump. Does this become problematic reconciling the module version with the spec version? What is the thinking here?
grant commentedon Apr 28, 2020
Good questions.
The npm version (CloudEvent client) and specversion may differ. As such, the npm module version may be higher than the CE spec (they should be unrelated). We don't really expect the Node client to only be changed when there's a major release in the CE spec of course.
I've updated the exact sample to be more consistent with Go and normal versioning. When
v2comes out, I would assume youimportorrequirev2.fabiojose commentedon Apr 29, 2020
No problems @lance, we may have versions higher than spec. Now we are are a kit and we have our own lifecycle that supports some spec versions and features.
lance commentedon May 12, 2020
@grant @fabiojose should we consider trying to get this in for a 2.0.0 release? I think it's a nice change, and since we are making big API changes with the upcoming release, this might be the place for it. My only concern is that this is probably a pretty big change, touching lots of the underlying code. It might be a big PR that takes some time to hash through. WDYT?
grant commentedon May 12, 2020
@lance Glad you like the proposed change.
So long as there are tests, any number of PRs to make the change are fine.