Azure / azure-functions-durable-js

JavaScript library for using the Durable Functions bindings
https://www.npmjs.com/package/durable-functions
MIT License
128 stars 46 forks source link

Remove constructor from declarations of `DurableClient` and `DurableOrchestrationContext` classes #471

Closed hossam-nasr closed 1 year ago

hossam-nasr commented 1 year ago

Related to #458 and #352. DurableClient and DurableOrchestrationContext are two classes that really should be interfaces. Changing them to interfaces now would be a breaking change, however. One of the reasons they shouldn't be classes is because users shouldn't call their constructors. They are passed instead passed down objects of this type from the Durable SDK.

This is the type signature of the DurableClient constructor:

constructor(clientData: OrchestrationClientInputData);

which relies on the OrchestrationClientInputData class:

constructor(
        public taskHubName: string,
        public creationUrls: HttpCreationPayload,
        public managementUrls: HttpManagementPayload,
        public baseUrl?: string,
        public requiredQueryStringParameters?: string,
        public rpcBaseUrl?: string
    ) {}

This is the constructor for DurableOrchestrationContext:

constructor(
        state: HistoryEvent[],
        instanceId: string,
        currentUtcDateTime: Date,
        isReplaying: boolean,
        parentInstanceId: string | undefined,
        longRunningTimerIntervalDuration: string | undefined,
        maximumShortTimerDuration: string | undefined,
        defaultHttpAsyncRequestSleepTimeMillseconds: number | undefined,
        schemaVersion: ReplaySchema,
        input: unknown,
        taskOrchestratorExecutor: TaskOrchestrationExecutor
    );

Exposing these two constructors, would force us to also expose these otherwise non-public-facing types/classes:

There might be others that I missed too. All these would become public types, meaning any breaking changes to their type signatures (from a change in how the SDK works internally) would become a breaking change. Since users shouldn't be calling constructors on those types anyway, we should not expose the constructor in their class declaration in the interest of #352

ejizba commented 1 year ago

Since users shouldn't be calling constructors on those types anyway, we should not expose the constructor in their class declaration

I feel like there's valid reasons to call random class constructors for testing purposes. That being said, I think #352 is reason enough to remove these for preview. We can always reassess if people ask for them

hossam-nasr commented 1 year ago

Closed by #472