Review fixes

Sushisource · Sushisource · commit 37bcf271e7c1 · 2025-05-01T18:06:16.000-07:00
diff --git a/packages/common/src/worker-deployments.ts b/packages/common/src/worker-deployments.ts
@@ -1,4 +1,4 @@
-import { temporal } from '@temporalio/proto';
+import type { temporal } from '@temporalio/proto';
 import { makeProtoEnumConverters } from './internal-workflow';
 
 /**
diff --git a/packages/common/src/workflow-definition-options.ts b/packages/common/src/workflow-definition-options.ts
@@ -1,7 +1,7 @@
 import { VersioningBehavior } from './worker-deployments';
 
 /**
- * Options that can be used when defining a workflow via {@link defineWorkflowWithOptions}.
+ * Options that can be used when defining a workflow via {@link setWorkflowOptions}.
  */
 export interface WorkflowDefinitionOptions {
   versioningBehavior?: VersioningBehavior;
@@ -14,6 +14,5 @@ export type WorkflowDefinitionOptionsOrGetter = WorkflowDefinitionOptions | (()
  * A workflow function that has been defined with options from {@link WorkflowDefinitionOptions}.
  */
 export interface WorkflowFunctionWithOptions<Args extends any[], ReturnType> extends AsyncFunction<Args, ReturnType> {
-  __temporal_is_workflow_function_with_options: true;
-  options: WorkflowDefinitionOptionsOrGetter;
+  workflowDefinitionOptions: WorkflowDefinitionOptionsOrGetter;
 }
diff --git a/packages/common/src/workflow-options.ts b/packages/common/src/workflow-options.ts
@@ -260,5 +260,5 @@ export function extractWorkflowType<T extends Workflow>(
 /* eslint-disable @typescript-eslint/explicit-module-boundary-types */
 export function isWorkflowFunctionWithOptions(obj: any): obj is WorkflowFunctionWithOptions<any[], any> {
   if (obj == null) return false;
-  return obj.__temporal_is_workflow_function_with_options === true;
+  return Object.hasOwn(obj, 'workflowDefinitionOptions');
 }
diff --git a/packages/test/src/deployment-versioning-v1/index.ts b/packages/test/src/deployment-versioning-v1/index.ts
@@ -1,7 +1,7 @@
-import { setHandler, condition, defineWorkflowWithOptions, workflowInfo } from '@temporalio/workflow';
+import { setHandler, condition, setWorkflowOptions, workflowInfo } from '@temporalio/workflow';
 import { unblockSignal, versionQuery } from '../workflows';
 
-defineWorkflowWithOptions({ versioningBehavior: 'AUTO_UPGRADE' }, deploymentVersioning);
+setWorkflowOptions({ versioningBehavior: 'AUTO_UPGRADE' }, deploymentVersioning);
 export async function deploymentVersioning(): Promise<string> {
   let doFinish = false;
   setHandler(unblockSignal, () => void (doFinish = true));
@@ -11,12 +11,12 @@ export async function deploymentVersioning(): Promise<string> {
 }
 
 // Dynamic/default workflow handler
-export default defineWorkflowWithOptions({ versioningBehavior: 'PINNED' }, _default);
-async function _default(): Promise<string> {
+setWorkflowOptions({ versioningBehavior: 'PINNED' }, module.exports.default);
+export default async function (): Promise<string> {
   return 'dynamic';
 }
 
-defineWorkflowWithOptions(() => {
+setWorkflowOptions(() => {
   // Need to ensure accessing workflow context still works in here
   workflowInfo();
   return {
diff --git a/packages/test/src/deployment-versioning-v2/index.ts b/packages/test/src/deployment-versioning-v2/index.ts
@@ -1,7 +1,7 @@
-import { setHandler, condition, defineWorkflowWithOptions } from '@temporalio/workflow';
+import { setHandler, condition, setWorkflowOptions } from '@temporalio/workflow';
 import { unblockSignal, versionQuery } from '../workflows';
 
-defineWorkflowWithOptions({ versioningBehavior: 'PINNED' }, deploymentVersioning);
+setWorkflowOptions({ versioningBehavior: 'PINNED' }, deploymentVersioning);
 export async function deploymentVersioning(): Promise<string> {
   let doFinish = false;
   setHandler(unblockSignal, () => void (doFinish = true));
diff --git a/packages/test/src/deployment-versioning-v3/index.ts b/packages/test/src/deployment-versioning-v3/index.ts
@@ -1,7 +1,7 @@
-import { setHandler, condition, defineWorkflowWithOptions } from '@temporalio/workflow';
+import { setHandler, condition, setWorkflowOptions } from '@temporalio/workflow';
 import { unblockSignal, versionQuery } from '../workflows';
 
-defineWorkflowWithOptions({ versioningBehavior: 'AUTO_UPGRADE' }, deploymentVersioning);
+setWorkflowOptions({ versioningBehavior: 'AUTO_UPGRADE' }, deploymentVersioning);
 export async function deploymentVersioning(): Promise<string> {
   let doFinish = false;
   setHandler(unblockSignal, () => void (doFinish = true));
diff --git a/packages/test/src/helpers-integration.ts b/packages/test/src/helpers-integration.ts
@@ -137,7 +137,7 @@ export function makeTestFunction(opts: {
     createTestContext: async (_t: ExecutionContext): Promise<Context> => {
       let env: TestWorkflowEnvironment;
       if (process.env.TEMPORAL_SERVICE_ADDRESS) {
-        env = await TestWorkflowEnvironment.createExistingServer({
+        env = await TestWorkflowEnvironment.createFromExistingServer({
           address: process.env.TEMPORAL_SERVICE_ADDRESS,
         });
       } else {
diff --git a/packages/test/src/helpers.ts b/packages/test/src/helpers.ts
@@ -215,10 +215,10 @@ export class TestWorkflowEnvironment extends RealTestWorkflowEnvironment {
     });
   }
 
-  static async createExistingServer(
+  static async createFromExistingServer(
     opts?: ExistingServerTestWorkflowEnvironmentOptions
   ): Promise<TestWorkflowEnvironment> {
-    return RealTestWorkflowEnvironment.createExistingServer(opts);
+    return RealTestWorkflowEnvironment.createFromExistingServer(opts);
   }
 }
 
diff --git a/packages/testing/src/index.ts b/packages/testing/src/index.ts
@@ -151,7 +151,7 @@ export type LocalTestWorkflowEnvironmentOptions = {
 };
 
 /**
- * Options for {@link TestWorkflowEnvironment.createExistingServer}
+ * Options for {@link TestWorkflowEnvironment.createFromExistingServer}
  */
 export type ExistingServerTestWorkflowEnvironmentOptions = {
   /** If not set, defaults to localhost:7233 */
@@ -302,7 +302,7 @@ export class TestWorkflowEnvironment {
     });
   }
 
-  static async createExistingServer(
+  static async createFromExistingServer(
     opts?: ExistingServerTestWorkflowEnvironmentOptions
   ): Promise<TestWorkflowEnvironment> {
     return await this.create({
diff --git a/packages/worker/src/utils.ts b/packages/worker/src/utils.ts
@@ -33,12 +33,12 @@ export function convertToParentWorkflowType(
 export function convertDeploymentVersion(
   v: coresdk.common.IWorkerDeploymentVersion | null | undefined
 ): WorkerDeploymentVersion | undefined {
-  if (v == null) {
+  if (v == null || v.buildId == null) {
     return undefined;
   }
 
   return {
-    buildId: v.buildId ?? '',
+    buildId: v.buildId,
     deploymentName: v.deploymentName ?? '',
   };
 }
diff --git a/packages/workflow/src/worker-interface.ts b/packages/workflow/src/worker-interface.ts
@@ -79,24 +79,28 @@ export function initRuntime(options: WorkflowCreateOptionsInternal): void {
     const workflowFn = mod[activator.info.workflowType];
     const defaultWorkflowFn = mod['default'];
 
-    if (isWorkflowFunctionWithOptions(workflowFn)) {
-      activator.workflow = workflowFn;
-      if (typeof workflowFn.options === 'object') {
-        activator.versioningBehavior = workflowFn.options.versioningBehavior;
+    if (typeof workflowFn === 'function') {
+      if (isWorkflowFunctionWithOptions(workflowFn)) {
+        activator.workflow = workflowFn;
+        if (typeof workflowFn.workflowDefinitionOptions === 'object') {
+          activator.versioningBehavior = workflowFn.workflowDefinitionOptions.versioningBehavior;
+        } else {
+          activator.workflowDefinitionOptionsGetter = workflowFn.workflowDefinitionOptions;
+        }
       } else {
-        activator.workflowDefinitionOptionsGetter = workflowFn.options;
+        activator.workflow = workflowFn;
       }
-    } else if (isWorkflowFunctionWithOptions(defaultWorkflowFn)) {
-      activator.workflow = defaultWorkflowFn;
-      if (typeof defaultWorkflowFn.options === 'object') {
-        activator.versioningBehavior = defaultWorkflowFn.options.versioningBehavior;
+    } else if (typeof defaultWorkflowFn === 'function') {
+      if (isWorkflowFunctionWithOptions(defaultWorkflowFn)) {
+        activator.workflow = defaultWorkflowFn;
+        if (typeof defaultWorkflowFn.workflowDefinitionOptions === 'object') {
+          activator.versioningBehavior = defaultWorkflowFn.workflowDefinitionOptions.versioningBehavior;
+        } else {
+          activator.workflowDefinitionOptionsGetter = defaultWorkflowFn.workflowDefinitionOptions;
+        }
       } else {
-        activator.workflowDefinitionOptionsGetter = defaultWorkflowFn.options;
+        activator.workflow = defaultWorkflowFn;
       }
-    } else if (typeof workflowFn === 'function') {
-      activator.workflow = workflowFn;
-    } else if (typeof defaultWorkflowFn === 'function') {
-      activator.workflow = defaultWorkflowFn;
     } else {
       const details =
         workflowFn === undefined
diff --git a/packages/workflow/src/workflow.ts b/packages/workflow/src/workflow.ts
@@ -1593,14 +1593,12 @@ export function allHandlersFinished(): boolean {
 }
 
 /**
- * Can be used to alter or define workflow functions with certain options specified at definition
- * time. In order to ensure that workflows are loaded properly by their name, you typically will not
- * need to use the return value of this function.
+ * Can be used to alter workflow functions with certain options specified at definition time.
  *
  * @example
  * For example:
  * ```ts
- * defineWorkflowWithOptions({ versioningBehavior: 'pinned' }, myWorkflow);
+ * setWorkflowOptions({ versioningBehavior: 'PINNED' }, myWorkflow);
  * export async function myWorkflow(): Promise<string> {
  *   // Workflow code here
  *   return "hi";
@@ -1610,29 +1608,25 @@ export function allHandlersFinished(): boolean {
  * @example
  * To annotate a default or dynamic workflow:
  * ```ts
- * export default defineWorkflowWithOptions({ versioningBehavior: 'pinned' }, myDefaultWorkflow);
- * async function myDefaultWorkflow(): Promise<string> {
+ * export default async function (): Promise<string> {
  *   // Workflow code here
  *   return "hi";
  * }
+ * setWorkflowOptions({ versioningBehavior: 'PINNED' }, module.exports.default);
  * ```
  *
  * @param options Options for the workflow defintion, or a function that returns options. If a
  * function is provided, it will be called once just before the workflow function is called for the
- * first time.
+ * first time. It is safe to call {@link workflowInfo} inside such a function.
  * @param fn The workflow function.
- * @returns The same passed in workflow function, with the specified options applied. You can export
- * this function to make it available as a workflow function.
  */
-export function defineWorkflowWithOptions<A extends any[], RT>(
+export function setWorkflowOptions<A extends any[], RT>(
   options: WorkflowDefinitionOptionsOrGetter,
   fn: (...args: A) => Promise<RT>
-): WorkflowFunctionWithOptions<A, RT> {
-  const wrappedFn = Object.assign(fn, {
-    options,
-    __temporal_is_workflow_function_with_options: true as const,
+) {
+  Object.assign(fn, {
+    workflowDefinitionOptions: options,
   });
-  return wrappedFn;
 }
 
 export const stackTraceQuery = defineQuery<string>('__stack_trace');

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-import { temporal } from '@temporalio/proto';`
	`1`	`+import type { temporal } from '@temporalio/proto';`
`2`	`2`	`import { makeProtoEnumConverters } from './internal-workflow';`
`3`	`3`
`4`	`4`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -260,5 +260,5 @@ export function extractWorkflowType<T extends Workflow>(`
`260`	`260`	`/* eslint-disable @typescript-eslint/explicit-module-boundary-types */`
`261`	`261`	`export function isWorkflowFunctionWithOptions(obj: any): obj is WorkflowFunctionWithOptions<any[], any> {`
`262`	`262`	`if (obj == null) return false;`
`263`		`- return obj.__temporal_is_workflow_function_with_options === true;`
	`263`	`+ return Object.hasOwn(obj, 'workflowDefinitionOptions');`
`264`	`264`	`}`
Original file line number	Diff line number	Diff line change
`@@ -215,10 +215,10 @@ export class TestWorkflowEnvironment extends RealTestWorkflowEnvironment {`
`215`	`215`	`});`
`216`	`216`	`}`
`217`	`217`
`218`		`- static async createExistingServer(`
	`218`	`+ static async createFromExistingServer(`
`219`	`219`	`opts?: ExistingServerTestWorkflowEnvironmentOptions`
`220`	`220`	`): Promise<TestWorkflowEnvironment> {`
`221`		`- return RealTestWorkflowEnvironment.createExistingServer(opts);`
	`221`	`+ return RealTestWorkflowEnvironment.createFromExistingServer(opts);`
`222`	`222`	`}`
`223`	`223`	`}`
`224`	`224`
Original file line number	Diff line number	Diff line change
`@@ -33,12 +33,12 @@ export function convertToParentWorkflowType(`
`33`	`33`	`export function convertDeploymentVersion(`
`34`	`34`	`v: coresdk.common.IWorkerDeploymentVersion \| null \| undefined`
`35`	`35`	`): WorkerDeploymentVersion \| undefined {`
`36`		`- if (v == null) {`
	`36`	`+ if (v == null \|\| v.buildId == null) {`
`37`	`37`	`return undefined;`
`38`	`38`	`}`
`39`	`39`
`40`	`40`	`return {`
`41`		`- buildId: v.buildId ?? '',`
	`41`	`+ buildId: v.buildId,`
`42`	`42`	`deploymentName: v.deploymentName ?? '',`
`43`	`43`	`};`
`44`	`44`	`}`