Optionally typed VirtualObject state keys

[mupperton]

Motivation in Discord channel: https://discord.com/channels/1128210118216007792/1255899794337959939

Due to TS strictness (good thing) this needed an unconventional default value for the Generic TState, which would be used to control whether the typings should follow the existing approach of ctx.get<TYPE> or should infer types from the provided TState - I was hoping to just use null but null doesn't satisfy the type Record<string, unknown>...

I haven't made any changes to context_impl.ts - not sure if needed? Due to it not specifying the TState type, it uses the existing behaviour anyway from a type perspective - let me know

Usage (screenshot taken from proposal in Discord channel):

[igalshilman]

Thanks @mupperton for opening this PR, will take a look!

mean while I've noticed that some lint checks are failing, you may want to run npm run format and push.

also fyi, there is a npm run verify that will try to build, run tests, check for linting errors etc'

[mupperton]

I've noticed that some lint checks are failing

Looks like unexpected use of any in the Record<string, any>

Seems that Record<string, unknown> does the job too from a quick test, so trying that

[igalshilman]

n the Record<string, any>

unknown doesn't seem to work for me, but any does. You can try playing around with the object.ts in the examples package.

[igalshilman]

Actually it works for type State = { .. } but doesn't work for interface State {}

[igalshilman]

The second thing, that it works for me as a standalone handler, but does not work when I add this as an handler to the object. The reason is that the ObjectOpts doesn't support this atm.

I think that the next step is adding the same TState type parameter to the object and ObjectOpts function/type. And then we'd have it e2e.

[igalshilman]

Hi @mupperton, it turns out to be much more involved than initially thought, but still doable. If you want I can complete this based on your initial PR.

[mupperton]

Hi @mupperton, it turns out to be much more involved than initially thought, but still doable. If you want I can complete this based on your initial PR.

Ah yes, can see now I missed the WorkflowContext too in addition to the things you listed

Happy for you to continue the work though - I've got some other commitments at work anyway

[mupperton]

Actually it works for type State = { .. } but doesn't work for interface State {}

Yeah, I think it's because the interface doesn't extend Record, but using anything else, like object or { [key: string]: any } instead of Record would give me a "key" type of string | number and sometimes Symbol in there too - I think I was expecting to need to cast them back to string at the time, but as I never touched the concrete implementations, maybe that will work OK and support interfaces too if desired?

[igalshilman]

Hey @mupperton fyi, I've followup with this here #401 It starts with this PR but also threads in all the type parameters to support workflows/object and their shared handlers.

One thing that I wasn't able to do is to provide a type parameter for the entire object, like so:

object<MyState>({ ... })

Because, providing this type parameter requires providing all the other type parameters. So for now we will at least can type each handler individually (this might actually be desired to expose a subset of the state for some handlers) But I'll definitely take a look at ways to type the entire object / and more ( properties, schema etc')

[mupperton]

Hey @igalshilman - thanks for this, will try out soon!

One thing that I wasn't able to do is to provide a type parameter for the entire object, like so:

object<MyState>({ ... })

Yeah I think this is probably too much of an ask as you say, but also because any handler defined in another file from the object itself will never have any awareness of the object without some re-importing of the object's type etc which could get messy, but the current solution is more than enough I believe for those that want to opt-in to this for now