-
Notifications
You must be signed in to change notification settings - Fork 89
Conversation
🦋 Changeset detectedLatest commit: a18ca78 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
setAlarm( | ||
arg2: Date, | ||
options?: DurableObjectStorageOperationsSetAlarmOptions | ||
scheduledTime: Date, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this right? getAlarm
returns a number
, but setAlarm
takes a Date
? Should setAlarm
take number | Date
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setAlarm
does take number | Date
, I guess we need to update the type generation script to understand this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setAlarm(kj::Date scheduledTime
Are you sure about that Matt? The type generation script is pretty stable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason we don't return a Date from getAlarm
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setAlarm(kj::Date scheduledTime
Are you sure about that Matt? The type generation script is pretty stable.
It's possible CFJS automatically casts numbers to kj::Date
s?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that Brendan's observation is valid. This API is asymmetric which is odd.
@xortive could you see what's causing this and see if we can fix it? thank you
setAlarm( | ||
arg2: Date, | ||
options?: DurableObjectStorageOperationsSetAlarmOptions | ||
scheduledTime: Date, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, should this be number | Date
?
Going to merge this because conversation is about API which isn't a great forum because this project represents the current state of the runtime and the types generated are actually correct. |
This PR includes the auto-generated types generated on
2022-04-27
.