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

Proposal: propagate function context into the 'this' type of functions #23554

Closed
DavidANeil opened this issue Apr 20, 2018 · 2 comments
Closed
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript

Comments

@DavidANeil
Copy link

DavidANeil commented Apr 20, 2018

Problem

TypeScript's type system is bad at preserving type information of transformed types (such as those created with the readonly modifier)

Examples of Current Behavior

Losing readonly Modifiers

class TestClass {
  constructor(public member: number) { }

  public doNothing() {
    return this;
  }
}

const tester: Readonly<TestClass> = new TestClass(1);

tester.member = 2; // Error: Cannot assign to 'member' because it is a constant or a read-only property.
tester.doNothing().member = 2; // No error.

Calling Functions Which Are Ignorant of readonly Modifiers

class TestClass {
  constructor(public member: number) { }

  public mutateSelf(newVal: number) {
    this.member = newVal;
  }
}

const tester: Readonly<TestClass> = new TestClass(1);

tester.member = 2; // Error: Cannot assign to 'member' because it is a constant or a read-only property.
tester.mutateSelf(2); // No error.

Calling functions with possibly undefined values with strictNullChecks

class TestClass {
  constructor(public member: number) { }

  public hopefullyExists() {
    return this.member.toString();
  }
}

let tester: Partial<TestClass> = new TestClass(5);

if (tester.hopefullyExists) {
  tester.member.toString(); // Error: Object is possibly 'undefined'.
  tester.hopefullyExists(); // No error.
}

These problems are caused because functions are context agnostic.
The first problem can be solved as a specific case in a few different ways.

// Annotate the `this` type, for specifically Readonly.
public doNothing(this: Readonly<this>);

// or make the `this` type a generic.
public doNothing<T extends this>(this: T);

These solutions are insufficient because they require

  1. Large work on the side of code authors to maintain.
  2. They are only possible in this case because of unexpected bivariance in assignability with the readonly property.

What Makes a Context Different

There are currently only 2 modifiers to a type in TypeScript

  • readonly which marks a property as read-only, but does not change assignability.
  • ? which marks the existence of the property as optional.

JavaScript also allows any function to be called with any context with the Function#call, Function#bind, and Function#apply commands. There is no current enforcement in the type system that these are valid calls.

Core Proposal

To solve these issues, a function must be able to give more control over its context; and for this to be viable in large projects, the allowable context must be determinable by the type checker.

The Implementation Solution

When an implementation of a function is known, determining the context's from which it can be called should be fairly trivial for the type checker. A brief overview of what that would look like:

  1. If the this value is never used, then the context can be any.
  2. For every use of this, determine if the member is accessed, add it to a set of read members.
    2.1. If it was read in a way that would be legal if it were an optional member, then add it to the optional set.
    2.2. Determine if the member is written to, add it to set of written members.
  3. Then, emit the function type declaration as
aFunction<this extends 
    {+readonly [R in read]: this[R]} &
    {-readonly [W in written]: this[W]} &
    {[O in optional]+?: this[O]}
>();

The Declaration Solution

Declaration files not generated by the type checker would be able to (and already can) add their own explicit context to a function. While the type checker has the opportunity to be very detailed in the required context, a hand-curated declarations file would be best served by using more broad strokes, such as:

aFunction<this extends Partial<this>>();

this In Generics

Currently the this type cannot be used in theConstraint portion of a generic TypeParameter. This would need to not be the case for this implementation to work in the wild.

When the keyword this appears as the BindingIdentifier in a TypeParameter it is shorthand for the following:

aFunction<this extends T>();
// Is equivalent to:
aFunction<CTX extends T>(this: CTX);

The reason for not just using the equivalent is because the this extends T allows the type checker to also assert that this extends T, which is confusing only because of the heavy use of this. A few examples:

interface RandomInterface {
  randomMember: number;
}

class ClassA {
 public member: number;
  badFunc<this extends RandomInterface>(); //Error: ' ClassA' does not extend '{randomObject: number}' 
  goodFunc<this extends Partial<RandomInterface>>(this: RandomInterface); // Succeeds: 'RandomInterface' extends 'Partial<RandomInterface>'
  normalFunc<this extends Partial<this>>(); // Succeeds: 'ClassA' extends 'Partial<ClassA>'
}

In that example goodFunc is using the already existing method of marking an explicit this type for a function.
The this extends Partial<RandomInterface> could have been written as this extends Partial<this> for the same meaning.

Compiler Flag

Because this change could cause existing code to not compile, it should be added behind a compiler flag strictContextChecks, this would affect if the declarations files that are output from implementations would include this context, also if the compiler was adding them implicitly for type checking a single project.

JavaScript Emit

None of the proposed changes would have any effect on the JavaScript emit.

Additional Comments

Until readonly a: number is not assignable to a: number then most issues with readonly are not solvable in a satisfying way.

Related Issues

@DavidANeil DavidANeil changed the title Proposal: type check functions against the transformed 'this' type of the calling context Proposal: propagate function context into the 'this' type of functions Apr 20, 2018
@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 Apr 20, 2018
@DavidANeil
Copy link
Author

A large part of this proposal was built off a faulty premise. I have updated the proposal based on what is hopefully correct information.
This has made the proposal much simpler, though it doesn't solve what I originally set out to solve.

This proposal combined with what is currently proposed as strictReadonlyChecks would be a solution that requires minimal effort to solve a lot of issues surrounding this.

@RyanCavanaugh RyanCavanaugh added Declined The issue was declined as something which matches the TypeScript vision and removed Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature labels Jun 24, 2021
@RyanCavanaugh
Copy link
Member

A lot of this is doable today now, e.g. the first example is an error if doNothing is declared with sufficient ceremony

public doNothing<T>(this: T): T {

Other investigations into this have shown that we lose way too much performance trying to model these effects in every case, and there doesn't seem to be much demand for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

2 participants