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

Refine type checking for class accessor when using decorator #52559

Open
5 tasks done
weareoutman opened this issue Feb 2, 2023 · 7 comments
Open
5 tasks done

Refine type checking for class accessor when using decorator #52559

weareoutman opened this issue Feb 2, 2023 · 7 comments
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@weareoutman
Copy link

Suggestion

πŸ” Search Terms

decorator, class accessor, auto-accessor, types, null checks.

βœ… Viability Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.

⭐ Suggestion

If a class accessor decorator will always add an initial value for the decorated accessor, TypeScript should infer that the accessor is always non-null, even if no initial value in its original declaration.

πŸ“ƒ Motivating Example

See playground.

The decorator dec will always initialize the accessor with a non-null value, but TS reports an error on @dec accessor data: Wrapper<string>;. It says "Property 'data' has no initializer and is not definitely assigned in the constructor" when strictNullChecks is enabled.

function dec<This, Value>(
    target: ClassAccessorDecoratorTarget<This, Wrapper<Value>>,
    context: ClassAccessorDecoratorContext<This, Wrapper<Value>>,
): ClassAccessorDecoratorResult<This, Wrapper<Value>> {
    return {
        init(initialValue) {
            if (initialValue !== undefined) {
                return initialValue;
            }
            return {};
        }
    }
}

interface Wrapper<T = unknown> {
    value?: T;
}

class Test {
    @dec accessor data: Wrapper<string>;
}

image

Are there any workarounds, other than putting an ! after data, to make the type checking works?

πŸ’» Use Cases

@RyanCavanaugh
Copy link
Member

If a class accessor decorator will always add an initial value for the decorated accessor, TypeScript should infer that the accessor is always non-null

I'm not seeing how this would happen. In the by far most-common case, the source code of the decorator is not visible, so shouldn't really be thought of as a thing that can be analyzed. ! specifically exists for the case where you have the information that "someone else" will be initializing something for you; it's not a workaround.

@weareoutman
Copy link
Author

Is it possible to do that by checking the return type of init the decorator returns?

function dec1() {
  return {
    init(): NonNull {
      // ...
    }
  }
}

function dec2() {
  return {
    init(): NonNull | undefined {
      // ...
    }
  }
}

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature labels Feb 23, 2023
@RyanCavanaugh
Copy link
Member

@rbuckton any input?

@rbuckton
Copy link
Member

rbuckton commented Feb 23, 2023

My current thinking is that we would handle this via the init signature in the future. For example:

function dec() {
  return {
    init<T>(initialValue: T | undefined): T {
      ...
    }
}

Where the body of init would need to produce a value matching the constraint of T when initialValue is undefined.

Another use case might be a dependency injection system, where init has the signature () => T and produces a value based on an ambient composition container, or throws if one isn't found.

In the type system, we would need to infer the type of the property from the return type of init, and check that the type of the actual initializer is assignable to the type of the initialValue argument, as if you had written accessor data = dec(...).init(undefined).

@ef4
Copy link

ef4 commented Aug 17, 2023

When starting to try to adopt the accessor decorator support in TS 5.x in real-world use cases (including dependency injection) we hit this need immediately.

The decorator returning a type with init(): T is not the only way to guarantee initialization. get(): T has the same effect.

(A possibly-separate-but-related issue is that the type of the field -- whether considered initialized or not -- is unambiguously the Value type from ClassAccessorDecoratorResult<This, Value> and the lack of inference is painful.)

I'm not seeing how this would happen. In the by far most-common case, the source code of the decorator is not visible, so shouldn't really be thought of as a thing that can be analyzed.

This is a weird explanation when the whole raison d'Γͺtre of typescript is to carry type information from the place where a function is implemented to the place where it is consumed. The existing type signatures of decorator functions already contain enough information to infer whether get or init always return a T:

type GetReturns<Result> = Result extends ClassAccessorDecoratorResult<
  unknown,
  infer Value
>
  ? Result['get'] extends undefined
    ? unknown
    : Value
  : unknown;

type InferredAccessorType<Decorator> = Decorator extends (
  ...args: any
) => infer Result
  ? GetReturns<Result>
  : unknown;

type Example = InferredAccessorType<typeof myDecoratorFunction>;

@ghost
Copy link

ghost commented Dec 1, 2023

Are there any plans to resolve this issue?

@runspired
Copy link

An additional problem with the current implementation is that ClassAccessorDecoratorResult enforces that the accessor getter and setter utilize the same Value type.

interface ClassAccessorDecoratorResult<This, Value> {

    get?(this: This): Value;

    set?(this: This, value: Value): void;

    init?(this: This, value: Value): Value;
}

However, because the decorator can add a separate getter and setter, it is valid to have a getter/setter combo with different signatures. E.g.

interface ClassAccessorDecoratorResult<This, GetValue, SetValue> {

    get?(this: This): GetValue;

    set?(this: This, value: SetValue): void;

    init?(this: This, value: GetValue): GetValue;
}

For example, you might have a getter that returns an asynchronous value (Promise<T>) but which accepts T as its setter signature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants