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

'identity' modifier to indicate a function's parameter-less returns should be narrowed like a value #60948

Open
6 tasks done
JoshuaKGoldberg opened this issue Jan 10, 2025 · 19 comments

Comments

@JoshuaKGoldberg
Copy link
Contributor

JoshuaKGoldberg commented Jan 10, 2025

πŸ” Search Terms

indicate function returns identical value cached memoized signals

βœ… Viability Checklist

⭐ Suggestion

Many apps today are built on the concept of using small "getter" functions as wrappers around values. For example, Signals as implemented in Angular, Solid, and the stage 1 TC39 Signals proposal often look something like:

declare const value: () => string | undefined;

if (value() !== undefined) {
  console.log(value().toUpperCase());
}

Signals users have struggled with using them in TypeScript because, at present, there isn't a way to get that code block to type check without type errors. Signals users know that the result of value() must be string inside the if, but TypeScript doesn't have a way to note that the result should be type narrowed. Common workarounds today include !, ?., and refactoring to store intermediate values. All of which are at best unnecessary verbosity, and at worst conflict with frameworks.

Request: can we have a keyword -or, failing that, built-in / intrinsic type- to indicate that calls to a function produce a referentially equal, structurally unchanging value? In other words, that the function call (value()) should be treated by type narrowing as if it was just a variable reference (value)?

Proposal: how about an identity modifier keyword for function types that goes before the ()? It would be treated in syntax space similarly to other modifier keywords such as abstract and readonly.

πŸ“ƒ Motivating Example

When an identity function is called, it is given the same type narrowing as variables. Code like this would now type check without type errors, as if value was declared as const value: string | undefined:

declare const value: identity () => string | undefined;

if (value() !== undefined) {
  value();
  // Before: string | undefined
  // Now: string

  console.log(value().toUpperCase());
  // Before:  ~~~~~~~ Object is possibly 'undefined'.
  // Now: βœ…
}

Narrowing would be cleared the same as variables when, say, a new closure/scope can't be guaranteed to preserve narrowing:

declare const value: identity () => string | undefined;

if (value() !== undefined) {
  setTimeout(() => {
    value();
    // Still: string | undefined

    console.log(value().toUpperCase());
    // Still:   ~~~~~~~ Object is possibly 'undefined'.
  });
}

πŸ’» Use Cases

One difficult-to-answer design question is: how could identity handle functions with parameters? I propose the modifier not be allowed on function signaturess with parameters to start. It should produce a type error for now. The vast majority of Signals users wouldn't need signatures with parameters, so I don't think solidifying that needs to block this proposal. IMO that can always be worked on later.

Furthermore, it's common for frameworks to set up functions with a parameter-less "getter" signature and a single-parameter "setter" signature. I propose for an initial version of the feature, calling any other methods or setting to any properties on the type should clear type narrowing:

declare const value: {
  identity (): string | undefined;
  (newValue: string | undefined): void;
}

if (value() !== undefined) {
  value("...");

  value();
  // Still: string | undefined

  console.log(value().toUpperCase());
  // Still:   ~~~~~~~ Object is possibly 'undefined'.
}

More details on the difficulties of signals with TypeScript:

If a new modifier keyword isn't palatable, a fallback proposal could be a built-in type like Identity<T>. This wouldn't be a new utility type (FAQ: no new utility types); it'd be closer to the built-in template string manipulation types.

@fabiospampinato
Copy link

fabiospampinato commented Jan 10, 2025

Keeping the narrowing inside the if seems problematic if any function could be executing before our second call to value() here, as that could result in the type checker telling us something incorrect:

declare const value: () => string | undefined;

if (value() !== undefined) {
  whatever();
  console.log(value().toUpperCase());
}

Also this working only in getter/setter-style signals is potentially limiting, for example I use unified signals, with a single function that is both a getter and a setter, because there are many benefits to doing that in a typed codebase (which essentially prevents the common issues of this approach from happening), and if this worked only for getters it would be of no use to me, basically.

In general the deeper problem seems about somehow detecting when various types of narrowing should be invalidated, which seems very hard to express and very difficult for the type checker to check for.

Worth exploring this area though, as signals are getting more popular.

@Andarist
Copy link
Contributor

Keeping the narrowing inside the if seems problematic if any function could be executing before our second call to value() here, as that could result in the type checker telling us something incorrect:

The same already happens with property narrowing so this wouldn't be a new problem at all. This would just use the same tradeoffs as the ones mentioned in #9998

@RyanCavanaugh
Copy link
Member

Isn't the core problem in this example that value is incorrectly declared? If it were

declare const value: (() => string) | (() => undefined);

if (value() !== undefined) {
  console.log(value().toUpperCase());
}

Then there's a very straightforward path to adding a narrowing rule that allows a normal interpretation of narrowing value to the () => string constituent.

@RyanCavanaugh
Copy link
Member

Also an ELI5 explanation for why it's not correct to write const v = value(); would be appreciated

@alxhub
Copy link

alxhub commented Jan 10, 2025

Isn't the core problem in this example that value is incorrectly declared?

Such a type might work for the simple case, but isn't really generalizable. If you consider the most basic implementation of a signal-like type:

class Signal<T> {
  constructor(private value: T) {}

  get(): T { return this.value; }
  set(value: T): void { this.value = value }
}

it would be very difficult / infeasible to type get into a union of all possible function types expressing the different narrowed shapes of T, without knowing the specific T. This is also a combinatorial problem: if T is something like {id: string|number, value: Data|undefined}, both id and value can be narrowed independently and encoding that into the get type would result in 4 different function types (assuming that even works).

Also an ELI5 explanation for why it's not correct to write const v = value(); would be appreciated

It's overhead compared to the experience with plain properties. Sometimes you have multiple levels of operations or multiple reads, which would result in a proliferation of temporary variables.

More critically, while value() is stable in the sense that repeated calls will return the same value, it may not be side-effect free. In the following chain:

if (x()) {
  createNewContext();
  x().value;
}

it might be important to record that x() is invoked after createNewContext(). Extracting to a temporary variable fixes the narrowing, but may alter the behavior in a breaking way.

@RyanCavanaugh
Copy link
Member

A distributive conditional type would be correct, though you would only be able to narrow a union (string | number), not a specific value (if T is string you still couldn't narrow to 0)

type PossibleFuncs<T> = T extends unknown ? () => T : never;
declare class Signal<T> {
  constructor(value: T) {}

  get: PossibleFuncs<T>
  set(value: T): void;
}

@ryansolid
Copy link

ryansolid commented Jan 10, 2025

Since where the read(function execution) happens matters in Signals libraries hoisting is incredibly clunky for a lot of cases. Especially in templating. Like think of JSX where everything is an expression not a statement. Most templating languages are effectively similar. Signal libraries tend to be granular in their rendering so components/templates don't re-run on a whole. Only parts that change re-execute. So the Signal function needs to be accessed in a very specific scope to trigger the right execution.

Not sure of people's familiarity but this is why often these sort of libraries can't destructure props. Because you can't access the getters at the top of the component but instead in the expression closest to where they are used. Different problem but part of the same mechanisms that are present here.

You don't always have a place to define variables and access the signal that might need to be used way nested down in your template. Places that are conditionally rendered, or parts of loops. A map function atleast can be made a block statement to be fair but even inside it there will be nested expressions so it can become onerous.

This is a fundamental aspect of Signals and the more granular people leverage them the more inevitable it will come that it will be painful to try to hoist stuff.

@Monkatraz
Copy link

Monkatraz commented Jan 10, 2025

A distributive conditional type would be correct, though you would only be able to narrow a union (string | number), not a specific value (if T is string you still couldn't narrow to 0)

type PossibleFuncs = T extends unknown ? () => T : never;
declare class Signal {
constructor(value: T) {}

get: PossibleFuncs
set(value: T): void;
}

How do you actually write this in non-declaration code?

This fails to type, and doesn't actually put get on the prototype:

type PossibleFuncs<T> = T extends unknown ? () => T : never;

class Signal<T> {
  private value: T;

  constructor(value: T) {
    this.value = value;
  }

  get: PossibleFuncs<T> = () => {
    return this.value;
  }

  set(value: T): void {
    this.value = value;
  }
}

You can do this instead, but value is considered unused, and it looks a little absurd:

type PossibleFuncs<T> = T extends unknown ? () => T : never;

interface Signal<T> {
  get: PossibleFuncs<T>;
}

class Signal<T> {
  private value: T;

  constructor(value: T) {
    this.value = value;
  }

  set(value: T): void {
    this.value = value;
  }
}

Signal.prototype.get = function () {
  return this.value;
}

As a side-note, Solid calls these kinds of functions Accessors

@alxhub
Copy link

alxhub commented Jan 10, 2025

A distributive conditional type would be correct, though you would only be able to narrow a union (string | number), not a specific value (if T is string you still couldn't narrow to 0)

This also only works for types in the top-level union. If a property of T is itself a union, it can't be narrowed via selecting a specific variant of PossibleFuncs<T>.

@RyanCavanaugh
Copy link
Member

Wait, if the idea is that get "always returns the same value", why is there a set method in the first place? Doesn't this imply the need for additional syntax to identify which functions/methods invalidate the narrowing?

@Monkatraz
Copy link

Monkatraz commented Jan 10, 2025

Wait, if the idea is that get "always returns the same value", why is there a set method in the first place? Doesn't this imply the need for additional syntax to identify which functions/methods invalidate the narrowing?

I mean, this violates the spirit of your argument as well:

class Foo {
  value: number | null = null

  reset() {
    this.value = null
  }
}

const foo = new Foo()
foo.value = 123
foo.reset()

// not a type error
foo.value.toFixed()

@Monkatraz
Copy link

Monkatraz commented Jan 10, 2025

I suppose if you're building with explicitly TypeScript in mind the ideal implementation is something like this:

class Signal<T> {
  private _value: T;

  constructor(value: T) {
    this._value = value;
  }

  get value(): T {
    // ... signal tracking logic

    return this._value;
  }

  set value(value: T) {
    // ... signal tracking logic

    this._value = value;
  }
}

But this wouldn't work for e.g. Solid, which just has accessors like foo() with no property access. Also, most consumers of the signal library won't be given signals directly and will instead have something wrapped around them, like useState in React or createSignal in Solid, both of which do not return an object that you do property access on.

EDIT: Also, the explicit function call syntax (so signal.get()) gets across that the access actually has side effects a little better.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jan 10, 2025

I mean, this violates the spirit of your argument as well:

Okay, but it seems extremely nearsighted to ship a new CFA feature that is going to immediately run into another feature request before it's considered useful. Bringing up the current CFA trade-offs as relates to properties doesn't seem like a useful way to address this concern.

@Monkatraz
Copy link

Monkatraz commented Jan 10, 2025

Okay, but it seems extremely nearsighted to ship a new CFA feature that is going to immediately run into another feature request before it's considered useful. Bringing up the current CFA trade-offs as relates to properties doesn't seem like a useful way to address this concern.

You're not wrong, but I think the feature would still be helpful regardless. The trade-offs are a "known quantity" to me, and this doesn't introduce anymore of them, right? It would be basically identical to how getters behave (except with no setter, I guess)

EDIT: I think with this feature it would be important for the naming and docs to get across it's intended for accessors, so as long as you don't yield to anything, you will know what the value is and it can't change on you (...usually), like a property.

@Monkatraz
Copy link

Monkatraz commented Jan 10, 2025

As an interesting anecdote, Svelte's old style of stores basically handled this problem via the magic $store syntax:

<script>
  import { writable } from 'svelte/store';

  const value = writable<number | null>(null)

  // magic reactive store access
  if ($value !== null) {
    // $value is now typed as non-null
    console.log($value.toFixed())
  }
</script>

Kind of like Signals, Svelte's compiler would track where you used these magic accesses. The Svelte LSP would basically trick the TypeScript LSP into thinking $value was a real variable iirc, and so it would naturally handle narrowing. It would even handle you setting the store's value, which I'm not sure will ever be achievable with any kind of [get, set] = whatever() pattern without some pretty substantial additions to TypeScript.

EDIT: There is also an old references proposal for JS which also touches on this problem, specifically this issue about making the concept of references extensible. Food for thought.

@alxhub
Copy link

alxhub commented Jan 10, 2025

Wait, if the idea is that get "always returns the same value", why is there a set method in the first place? Doesn't this imply the need for additional syntax to identify which functions/methods invalidate the narrowing?

The idea is not that it always returns the same value, but that it has the same expectations as a property accessor - it can be assumed to be stable within the narrowing context of a conditional statement or expression.

For Angular, we see significant value in narrowing the getter type and don't view invalidation of this narrowing on .set as a requirement at all.

Even with property getters, narrowing on the setter has never really been sufficient. Many constructs have other methods which invalidate their getters, and it tends not to be an issue in practice.

Basically, we'd strongly prefer the convenience of narrowing the getter function regardless of whether that narrowing was invalidated by the setter.

@fabiospampinato
Copy link

fabiospampinato commented Jan 10, 2025

The idea is not that it always returns the same value, but that it has the same expectations as a property accessor - it can be assumed to be stable within the narrowing context of a conditional statement or expression.

I think that is a good summary for why this feature might be useful.

I don't know if it would actually have the "same" expectations though, because all property accesses are type-checked like that, but not all function calls would be type-checked like that, which seems inconsistent/confusing in a way type-checking property accesses isn't.


In general I think a significant problem regarding the utility of this feature is that a signal is not actually the smallest "possibly-reactive" unit, a function is the fundamental "possibly-reactive" unit.

Like let's say we have a component like this:

function Paragraph({value}: Props) {
  return <p>{value}</p>;
}

Or a primitive/hook like this:

function useDoubled(value: Value) {
  return () => unwrap(value) * 2;
}

Or a derivation somewhere that looks like this:

const doubled = () => value() * 2;

For the component and the hook in general you don't want to say that you accept only signal values, that would be ridiculous, and you don't want to say that you accept only signal values or primitives values either, that's still unnecessarily limiting and weird, what you really should say is that you accept a primitive value or a function to a primitive value, basically a non-reactive thing or a possibly-reactive wrapper to the thing, i.e. "if you give me a reactive version of this thing I support reacting to it".

For that derivation you don't want to create a signal because that's unnecessary overhead, every time "value" will change "doubled" will change too (assuming -0 and +0 don't matter here), and you are listening to a single signal. Wrapping that function in a memo/computed today would give you absolutely nothing other than verbosity and overhead.

Basically the problem is that the second you are dealing with plain functions this special narrowing wouldn't apply anymore, and you want to say you accept plain functions as inputs, because they are the fundamental "possibly-reactive" unit, so the usefulness of narrowing signal getters seems pretty limited.

What we actually want, ideally, is for TS to understand when the same function called again will return the same type as before because its return value depends only on the values of the signals it read the last time, and those values couldn't have possibly changed since the last call of the function.

What this feature would give us is instead special-casing type checking for signal getters like property accesses, which is a very different beast.

Maybe it's still a useful one though?

Personally I'm not convinced it would solve a big-enough slice of the problem to be worth supporting, but the problem it is trying to solve is a real problem.

@fabiospampinato
Copy link

fabiospampinato commented Jan 10, 2025

Worth mentioning also that even if you say that you accept only primitives or signals to primitives, which again is overly limiting but let's pretend it's fine, you just can't reasonably take advantage of this special narrowing either, normally, because are you going to check if every value is a signal before doing something with it? No, you are going to want to have a function that unwraps possibly-reactive values, to delete this annoying branching, so the type narrowing of the signal would not be taken advantage of in many cases.

This would only really largely address the problem when one accepts only signals (not unreactive values, nor plain functions to unreactive values), and one makes only signals (not plain functions), which presumably everybody should agree nobody should be doing? That means, just to look at it syntactically, instead of writing <Foo active /> or <Foo active={true} /> you'd have to write something like <Foo active={createSignal(true)[0]} /> in today's Solid code, if Solid's JSX worked like React's JSX, or the equivalent useFoo(createSignal(true)[0]) if we are talking about hooks, which are the other side of this which people kinda pretend doesn't exist.

@felix-quotez
Copy link

felix-quotez commented Jan 11, 2025

For what it's worth, if the below would be working for primitive types (including unions, intersections, null, undefined--but not object types), wouldn't it already be a big step forward for signals as implemented in SolidJS?

type PossibleFuncs<T> = T extends unknown ? () => T : never;
const value: PossibleFuncs<string | undefined>;
if (value() !== undefined) {
  console.log(value().toUpperCase());
}

By casting the signal getter (as returned from createSignal) to PossibleFuncs SolidJS code could nicely expresses to TS that the getter returns a "stable" result.

It might not be required to solve use cases with object types, because they are covered by SolidJS's store (which is using proxies and getters) and, I think, it already gets desired type narrowing. Would adding this limitation to primitive types allow to get combinatorial complexity problem under control?

Edit: Seems related Method return type cannot be used as discriminant

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

8 participants