Skip to content

Instantly share code, notes, and snippets.

@swyxio
Last active October 25, 2021 18:53
Show Gist options
  • Select an option

  • Save swyxio/0dd6e8fd87d671ff6e79dc077372c94d to your computer and use it in GitHub Desktop.

Select an option

Save swyxio/0dd6e8fd87d671ff6e79dc077372c94d to your computer and use it in GitHub Desktop.
workflow apis tweak.md

now that we have a cohesive look at workflow APIs, some issues remain:

  • createActivityHandle isnt really a handle, in the way that createWorkflowHandle and createChildWorkflowHandle are. it returns a function that you call.
    • users are confused by the proxy destructure which a very fancy way of doing type safety and ensuring consistent naming
  • defineSignal/Query dont add much value since they just create an object
    • extra setListener api that is doing the real work, basically 2 different functions branching by def.type

just taking another crack at api design.

simplified query and signal api

status quo

export const unblockSignal = wf.defineSignal('unblock');
export const isBlockedQuery = wf.defineQuery('isBlocked');

export async function unblockOrCancel() {
  let isBlocked = true;
  wf.setListener(unblockSignal, () => void (isBlocked = false));
  wf.setListener(isBlockedQuery, () => isBlocked);
  console.log('Blocked');
  try {
    await wf.condition(() => !isBlocked);
    console.log('Unblocked');
  }
  catch (err) {
    if (err instanceof wf.CancelledFailure) {
      console.log('Cancelled');
    }
    throw err;
  }
}

proposal

if we eliminate setListener...

// using exportable definitions
export const onSetState = useSignal<boolean>('setState');
export const getIsBlocked = useQuery<boolean>('isBlocked');

export async function unblockOrCancel() {
  let isBlocked = true;
  onSetState((newState) => void (isBlocked = newState)); // boolean
  getIsBlocked(() => isBlocked);
  // ...
}

if they dont like on (because of the subscribe implication) they can name it setStateHandler or BlockedQueryResolver or whatever they like... we dont prescribe the naming.

for those who

  • don't need to export the signaldef/querydefs for strong typing for invocation (eg in the nextjs example its pretty inconvenient to import the types from the temporal folder into the nextjs folder, most people wont even bother)
  • don't need to reassign the listener

this enables inlining and reduces naming need:

// using strings
export async function unblockOrCancel() {
  let isBlocked = true;
  useSignal('unblock', () => void (isBlocked = false));
  useQuery('isBlocked', () => isBlocked);
  // ...
}

renamed Activity api

status quo

import { createActivityHandle } from '@temporalio/workflow';
import type * as activities from './activities';

const { greet } = createActivityHandle<typeof activities>({
  startToCloseTimeout: '1 minute',
});

/** A workflow that simply calls an activity */
export async function example(name) {
  return await greet(name);
}
  • this is decent tbh, but for some people (who would like to use typescript, but are not typescript gods), <typeof activities> is a foreign language and hard to decipher.
  • i am worried that people will just copy paste this and not really intuitively understand how to manipulate activities to suit their code style.
  • it also requires people to barrel all types into a single activities file (not really, but people will treat it that way)... would be nice to let people componentize or combine as they wish

proposal

i'd like to make clear that this is "just" a function and that we are importing from worker that must have this activity registered.

import { useActivity } from '@temporalio/workflow';
import type greet from './activities/greet'; // very clear that barrel file is optional

const invokeGreet = useActivity<greet>('greet', {
  startToCloseTimeout: '1 minute',
  // retries, etc
});

/** A workflow that simply calls an activity */
export async function example(name) {
  return await invokeGreet('world')
}

this means that you cant do the fancy multiple destructures, but hopefully usage will be much simplier because "less magic"...

import { useActivity, ActivityOptions } from '@temporalio/workflow';
import type foo, bar, baz from './activities'

const options: ActivityOptions =  {
  startToCloseTimeout: '1 minute',
  // retries, etc
}
const invokeFoo = useActivity<foo>('foo', options)
const invokeBar = useActivity<bar>('bar', options)
const invokeBaz = useActivity<baz>('baz', options)

/** A workflow that simply calls an activity */
export async function example(name) {
  await invokeFoo('world')
  await invokeBar('world')
  await invokeBaz(123, 345)
}
@lorensr
Copy link
Copy Markdown

lorensr commented Oct 19, 2021

Signals & queries

FWIW, I think handle and handler nouns are clearly different English & coding concepts and still prefer setHandler 🤷‍♂️ Mainly because handling seems more accurate about what the function is doing than listening, particularly in the case of queries, but also because listeners sounds like I can set multiple listeners, and handler doesn't.

We need to know:

  • string name of the signal/query
  • the listener function
  • the type the listener returns (for recommended TS use)
  • whether signal or query (ideally, but not necessary, if we're okay setting listener for both signal and query of same name)

Current way of getting all that info without repetition:

setListener(defineQuery<GameInfo>('getGameInfo'), () => gameInfo);

Would this work? It reads nicer to me:

setQueryListener<GameInfo>('getGameInfo', () => gameInfo)

Activities

I like in this one that I can call the function greet:

// A
import types * as activities from './activities';

const greet = useActivity<typeof activities.greet>('greet', { startToCloseTimeout: '1m' });

The destructuring complexity is gone. The import type * as and typeof complexity remains. For those two things, are these the options?

// B
import type greet from './activities/greet';

const greetFn = useActivity<typeof greet>('greet', { startToCloseTimeout: '1m' });
// C
import { GreetType } from './activities/greet';
// or
import type { GreetType } from './activities/greet';

const greet = useActivity<GreetType>('greet', { startToCloseTimeout: '1m' });

With B I don't like greetFn, and with C I don't like having to define GreetType. So I prefer A and adding to the useActivity docs what the activities file import/export options are.

@swyxio
Copy link
Copy Markdown
Author

swyxio commented Oct 25, 2021

setHandler('unblock', () => void (isBlocked = false)); // effectively only a signal
setHandler('isBlocked', () => isBlocked); // signal AND query
// problems - some stuff is illegal in queries

// typesafe
const myQuery = defineQuery<[boolean]>('isBlocked') // name + type
setListener(myQuery, () => isBlocked) 


// loren typesafe
export type GameInfo = boolean
setQueryListener<GameInfo>('getGameInfo', () => gameInfo)

const myQuery = defineQuery<[boolean]>('isBlocked') // name + type
const mySignal = defineSignal<[boolean]>('isBlocked') // name + type

setHandler(mySignal, () => void (isBlocked = false));
setHandler(myQuery, () => isBlocked);



setHandler<someType>({ type: 'signal', name: 'plsBlock' }), () => void (isBlocked = false));
setHandler<someType>({ type: 'query', name: 'isBlocked' }), () => void (isBlocked = false));

@lorensr
Copy link
Copy Markdown

lorensr commented Oct 25, 2021

Sounded like we have two things to decide:

  1. Do we want to provide the single-activity option instead of or in addition to? (Only benefit is no destructuring)

Sounded like no?

  1. If multiple, what do we want to call it? I like useActivity or useActivities or both.
import type * as activities from './activities';

// single
const greet = useActivity<typeof activities.greet>('greet', { startToCloseTimeout: '1m' });
const salute = useActivity<typeof activities.salute>('salute', { startToCloseTimeout: '1m' });

// multiple "useActivities"
const { greet, salute } = useActivities<typeof activities>({
  startToCloseTimeout: '1 minute',
});

// multiple "createActivityFunctions"
const { greet, salute } = createActivityFunctions<typeof activities>({
  startToCloseTimeout: '1 minute',
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment