nylas / nylas-nodejs

A NodeJS wrapper for the Nylas REST API for email, contacts, and calendar.
MIT License
169 stars 118 forks source link

Renaming of keys when serializing #413

Closed samuelgoldenbaum closed 1 year ago

samuelgoldenbaum commented 1 year ago

Describe the bug Not a bug, but really raising the issue about the serialization of classes and the renaming of keys – an lack of clear notes detailing this. The types defined do not match the serialized version, and keys are renamed according to the attributes on the class, which is simply a map of manually entered strings.

While it seems all the keys are simply renamed to snake case, this cannot be guaranteed as they are manually set. This means having to manually create a set of the dtos based on the attributes of the various Nylas models – an onerous chore.

Can we assume that the keys will always be renamed to snake case so we can automate this process in our projects?

The nodejs docs say:

// Return the first event
const event = await nylas.events.first();

// The following attributes are available for the event object
event.id
event.object
event.accountId
event.calendarId
event.messageId
event.title
event.description
event.owner
event.participants
event.readOnly
event.location
event.when
event.when.startTime
event.when.endTime
event.when.startDate
event.when.endDate
event.busy
event.status
event.iCalUID
event.metadata
event.conferencing
event.conferencing.provider
event.conferencing.details.url
event.conferencing.details.password
event.conferencing.details.meetingCode
event.conferencing.details.phone
event.conferencing.details.pin

To Reproduce stringify any model and view the results

const events: Event[] = await nylas.events.list({
        calendar_id,
        starts_before,
        starts_after
      })
      const payload = JSON.stringify(events)

Expected behavior Given the types defined, one would expect the names of the property keys to be consistent with the defined type. Or at least explicitly state this is not the case, so developers are aware of it.

SDK Version: 6.6.1

mrashed-dev commented 1 year ago

Hey @samuelgoldenbaum thanks for opening this issue. The serialization is currently done manually but you got it correct, we just use camelCase throughout the codebase and when serialization to/deserialization from the API we deal with snake_case fields as that's what the API is set to. We are looking into more efficient ways to do this, but for now you can safely assume that fields from the API are snake_case and the corresponding keys in the SDK models are camelCase.

However, as an SDK end user, you would be dealing with the model's keys directly as opposed to what's serialized for the API. I do see that you are using JSON.stringify(events), which converts it to snake_case for the API, would you be interested in having a second function that would serialize the objects in camelCase?

samuelgoldenbaum commented 1 year ago

Apologies for the delay; been on VAC. The variance should be documented more clearly – our devs simply followed the Noed.js examples and the docs don't mention this – ended up creating issues through the tiers and needed to look through the source code to clarify. In addition, some clarity that the conversion is actually to snake_case as it seems all the corresponding converted values are entered manually now.

mrashed-dev commented 1 year ago

@samuelgoldenbaum Thank you so much for your feedback. It's been logged internally and we're going to explore this (and deserialization in general) in the upcoming major release as we're going to be making tweaks to the models and underlying core functionality anyways.

For the current 6.x versions, you can assume that all of our models get serialized from camelCase to snake_case when being serialized from the Node object to a JSON for the Nylas API payload. It's done manually as of right now, but the convention on the Nylas API for all of our endpoints are snake_case. We'll look to eliminate this confusion via either automated serialization or with proper documentation. Thank you again for your feedback and patience on this.