statelyai / xstate-tools

Public monorepo for XState tooling
183 stars 40 forks source link

"Jump to definition" doesn't work with v5's `setup(...)` API #479

Open HerbCaudill opened 8 months ago

HerbCaudill commented 8 months ago

This feature advertised on the extension README doesn't seem to work with APIs recommended for setup in v5. My state machine is instantiated like this

const machine = setup({
  types: { ... },
  actions: { ... },
  guards: { ... })
.createMachine({
  context: { ... },
  // ... state machine definition
})

As far as I can tell:

In this screen capture, everywhere I click is a command-click, so should jump to definition if one is available.

https://github.com/statelyai/xstate-tools/assets/2136620/d1dae01a-c4d3-4af9-9e13-472f3e4b53a6

Andarist commented 8 months ago

We are in the process of rewriting the extension to improve v5 compatibility. This feature isn't exactly on our MVP list but the rewrite will allow us to implement this soon after we manage to cut that first MVP release.

HerbCaudill commented 8 months ago

That's great to hear! I think this specifically would make a big difference for adoption. My take as a developer FWIW is that XState's biggest DX challenge is the whole string-literals-that-refer-to-functions thing, and that's largely to do with the friction that comes from not having IDE support for going back and forth between the functions and their string references.

Andarist commented 8 months ago

I now really really wonder if we just couldn't improve this situation in TS itself.

It doesn't sound unreasonable for this to work:

declare function setup<TActions>(_: { actions: TActions }): {
  createMachine: (config: { entry: keyof TActions }) => void;
};

setup({
  actions: {
    doStuff: () => {},
    doMoreStuff: () => {},
  },
}).createMachine({
  // "go to definition" on the string here should bring us to the action's definition
  entry: "doStuff", 
});

Our situation is, of course, a little bit more complex and involves even more indirection... but making the above work could be the first step to improving our situation. All of this is highly related to this open ticket: https://github.com/microsoft/TypeScript/issues/49033

note for myself: I think there is a good chance that this could be improved by creating the origin type for Anonymous objects (and maybe some others too) in getLiteralTypeFromProperties. Later on, this information could be read by getDefinitionAtPosition from the contextual type, and at that point, it should be fairly straightforward to create the definition based on that. This would, at the very least, improve the situation when this contextual type is a union (that's why my example above intentionally has 2 actions and not just 1). Fixing this the same for objects with a single property would require introducing an additional concept to the compiler - literal string types with .origin. It's not impossible but it would require more work and there are memory/perf concerns related to this. I think that fixing this just for unions, especially for the PoC, would be good enough for the first iteration.