Add optional AlarmInvocationInfo parameter to Durable Object alarm() method #3224
Conversation
|
The generated output of |
jclee
force-pushed
the
jlee/add-alarm-param-type
branch
from
a568d59 to
5b877ca
Compare
| @@ -106,7 +106,7 @@ class DurableObject final: public Fetcher { | |||
|
|
|||
| JSG_TS_DEFINE(interface DurableObject { | |||
| fetch(request: Request): Response | Promise<Response>; | |||
| alarm?(): void | Promise<void>; | |||
| alarm?(alarmInfo?: AlarmInvocationInfo): void | Promise<void>; | |||
There was a problem hiding this comment.
I notice in our index.d.ts files, we define both a DurableObject interface at the top level and a DurableObject abstract class inside the cloudflare:workers module. I think the interface is intended to cover legacy uses of Durable Objects prior to the introduction of the class, and that extending the class is the way we recommend implementing Durable Objects in documentation and tutorials. I'm not sure whether it's important to add the parameter to the interface, or where that gets tested, but I went ahead and added it for completeness.
| @@ -467,7 +467,7 @@ interface Cloudflare { | |||
| } | |||
| interface DurableObject { | |||
| fetch(request: Request): Response | Promise<Response>; | |||
| alarm?(): void | Promise<void>; | |||
| alarm?(alarmInfo?: AlarmInvocationInfo): void | Promise<void>; | |||
There was a problem hiding this comment.
The changes to types/generated-snapshot/* were all generated by running the command recommended by a CI error message:
The generated output of @cloudflare/workers-types has been changed by this
PR. If this is intentional, run:
bazel build //types && rm -rf types/generated-snapshot && cp -r bazel-bin/types/definitions types/generated-snapshot
I assume it's doing the right thing by regenerating these snapshot files, but I'm not familiar with the intent.
| class TestAlarmObject extends DurableObject { | ||
| // Can declare alarm method consuming optional alarmInfo parameter | ||
| async alarm(alarmInfo?: AlarmInvocationInfo) { | ||
| if (alarmInfo !== undefined) { |
There was a problem hiding this comment.
FWIW, the current version of the runtime always passes this alarmInfo parameter, but early implementations did not, and it's likely that almost all Durable Object alarm() implementations in the wild do not currently declare the parameter. I think that means that the parameter needs to be declared alarmInfo? for backwards compatibility, although it is a bit unfortunate that this requires TS DOs to do this check for undefined before using the parameter.
I'm not sure if there is a better way to declare the method in the TS rpc.d.ts file such that it allows class implementers to either consume a (non-undefined) alarmInfo parameter or to omit it entirely.
Probably not a big deal in the common case, though, since most implementers probably won't be writing an alarm() handler that needs the info in the parameter?
There was a problem hiding this comment.
Dumb question: is there a way to set the alarmInfo as always passed as of a particular compatibility date?
There was a problem hiding this comment.
Not dumb -- I was wondering myself, but am not familiar with how we are doing versioning and backwards compatibility of TypeScript declarations.
I assume the purpose of the various types/generated-snapshot/* files might be to allow for compatibility-breaking changes in types, but I'm not sure if there is a correspondence to compatibility dates. I think marking the parameter as mandatory would not be backwards compatible, in the sense that it could cause TS to issue new errors or warnings for existing worker code -- either for alarm() handler definitions, or in cases where worker code tries to invoke alarm() handlers directly.
I suspect this feature will be infrequently used enough that it might not be worth breaking backwards compatibility for, by itself -- but if there comes a time when we're breaking backwards compatibility anyway, that might be a convenient time to make the parameter mandatory?
…le Object alarm() method The runtime passes in this parameter when invoking alarm(); the `retryCount` property gives a count of the number of previous times the runtime has tried to run this specific alarm event. The runtime has been passing this parameter to alarm() calls since it was originally introduced earlier in the year, in PR#1673, but the parameter declaration has not yet been added to the worker-types TypeScript declarations. This commit just updates the TypeScript declarations to include this parameter.
jclee
force-pushed
the
jlee/add-alarm-param-type
branch
from
5b877ca to
d0e1bd5
Compare
The runtime passes in this parameter when invoking alarm(); the
retryCountproperty gives a count of the number of previous times the runtime has tried to run this specific alarm event.The runtime has been passing this parameter to
alarm()calls since it was originally introduced earlier in the year, in PR #1673, but the parameter declaration has not yet been added to the worker-types TypeScript declarations. This PR just updates the TypeScript declarations to include this parameter.