Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typing issue with computed signals #18

Open
mindplay-dk opened this issue Aug 17, 2021 · 4 comments
Open

Typing issue with computed signals #18

mindplay-dk opened this issue Aug 17, 2021 · 4 comments

Comments

@mindplay-dk
Copy link

Consider an example like this, in TypeScript:

const state = signal({
  foods: ["pizza", "beer", "cake", "more pizza"],
  numFoods: wire($ => state.foods($).length),
});

There's a problem with this: inference for the type of numFoods fails, because it is dependent on the type of foods, which hasn't been inferred yet. TypeScript doesn't seem to allow cycles between inferred members. (Even though this is not technically a cycle if you consider the individual property types, TypeScript appears to try to resolve the types via the inferred type of the object rather than it's properties, resulting in a cycle... or something?)

I could manually type out an interface for the signal call, but that's a bother - explicitly passing the types doesn't work, because the type of state would still be inferred:

const state = signal<{ foods: string[], numFoods: number }>({
  foods: ["pizza", "beer", "cake", "more pizza"],
  numFoods: wire($ => state.foods($).length),
});

I could manually type out the entire return-type - that works, but it's a real bother:

const state: { foods: Signal<string[]>, numFoods: Signal<number> } = signal({
  foods: ["pizza", "beer", "cake", "more pizza"],
  numFoods: wire($ => state.foods($).length),
});

I could break them apart like this:

const state = signal({
  foods: ["pizza", "beer", "cake", "more pizza"],
});

const computed = signal({
  numFoods: wire($ => state.foods($).length),
});

That works, but two collections isn't what I wanted.

I could wrap them in a function and tease out the inference that way:

const state = (() => {
  const state = signal({
    foods: ["pizza", "beer", "cake", "more pizza"],
  });

  const computed = signal({
    numFoods: wire($ => state.foods($).length),
  });

  return { ...state, ...computed };
})();

That also works, but, yuck...

I'm having two thoughts here.

💭 Having to create multiple, unrelated states at once might not be a good idea. I've pointed this out before, and I know you don't agree, because you're attached to the idea of object properties turning into debug-friendly names. I'm still not convinced of that, because the names will get mangled, and this won't save you when things fail in production - I'd prefer to find a more reliable approach to debugging.

💭 Alternatively, maybe the recommended idiom is to separate state from computed state, like in the two last examples - maybe you can argue that separating mutable from computed state is "a good thing".

If so, maybe we could have a separate helper, similar to signal, for computed state only - this would just save you the repeated wire calls, when creating your derived states, but maybe it provides a more explainable concept?

const state = signal({
  foods: ["pizza", "beer", "cake", "more pizza"],
});

const computed = computedSignal({
  numFoods: $ => state.foods($).length,
});

I'm not particularly fond of letting implementation issues drive design - but in this case, I suppose you could argue it's good to separate mutable state from computed state? 🤔

(This issue is partially me thinking ahead to documentation/tutorials - it would be great if we didn't need to explain problems like this, and even better if users didn't have to run into them and look for a solution and explanation in the first place...)

@mindplay-dk
Copy link
Author

By the way, I've seen how you get around this in the documentation:

const state = signal({
  name: 'Deciduous Willow',
  nameReversed: wire(($): string =>
    state2.name($).split('').reverse().join()),
});

I understand how manually type-hinting the return type with ($): string avoids the need for inference, which wouldn't work - but this is just as difficult to explain as any of the options described above, and wouldn't be the first thing anyone thinks of. I am looking for something that works pretty much out-of-the-box...

@mindplay-dk
Copy link
Author

I started working on a TodoMVC implementation, and it turns out, separating state from computed doesn't really work when you have computed state derived from other computed state - you can see why:

https://codesandbox.io/s/haptic-todomvc-tir4k?file=/src/index.tsx

You basically have to start a new object anytime you have a computed dependency.

That or forego inference, which does not spark joy. 🤔

@nettybun
Copy link
Owner

This has been a huge issue for sure. I hacked it out in the TS playground way back in April and eventually hopped into the TS Discord to ask for help...

It's a necessary evil from how the TS compiler infers types: microsoft/TypeScript#43683

In #1 I wrote:

I thought it was worth it to settle on the version that (unfortunately) requires manually specifying the wire's return type - it's the best of many not great options due to limitations in the TS compiler.

Also, ever since I learned that it's possible to have JSX ternaries without a virtual-DOM or a compiler.... I really need to support that and not use when(). Doing this means I need to support non-lazy computeds and computeds that only write when a value is different than the saved value... tl;dr the signal API will need to be modular and broken up.

you're attached to the idea of object properties turning into debug-friendly names ... I'm still not convinced of that, because the names will get mangled, and this won't save you when things fail in production

Yes I am very stubborn about explicit names on signals and wires. It feels like an important part of what makes the framework, especially since - as someone who has worked in production development for hugely popular applications - it's very rare that I see devs use a debugger; a lot of people only read the error message and call stack from the console. Maybe you're better than that, and that's lovely, but I'm writing Haptic with my non-programmer friends in mind.

I'm still trying to find something that feels good to use.

@nettybun
Copy link
Owner

nettybun commented Aug 29, 2021

I played around with TS playground to come up with a way to stamp/brand/tag signals so their type is like Signal<"count", number> and they don't need to be defined using an object...

Haven't published anything formal yet but here's the idea: microsoft/TypeScript#45310 (comment)

Unusual syntax but might be a good tradeoff since using objects isn't good for us or for TS. I'm glad you came to all the same conclusions I had about how to compose objects into larger state objects but...

That also works, but, yuck...

Very.

To replace objects I've got an ok const state = pack(sig1, sig2, ... sigN) function in TS that does proper type checking to produce a state object which ✨ errors on duplicate signal names ✨ just like a real TS object would do. People might not even want to pack things into state objects and that's fine. It's kind of an old thing I borrow from React's class-based state models.

I'm going away on a roadtrip but will look at this more soon! Thank you tons for all the feedback! 💯 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants