microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.08k stars 12.37k forks source link

Allow "instance" private and protected class members that don't affect assignability #52119

Open justinfagnani opened 1 year ago

justinfagnani commented 1 year ago

Suggestion

Many classes use private and protected members for internal use only - that is, they always access them via this.field and never on other object with other.field.

For these classes it would be very useful to retain structural typing, instead of the effectively nominal typing you get with private and protected members currently.

This would be especially useful for avoiding compilation errors when programs have multiple copies of the same package installed (for the usual npm reasons) with classes that are actually compatible with each other, but aren't assignable according to TypeScript.

Could we add an instance modifier to class members that restricts the fields to only being access with this.?

Example:

class A {
  instance private _x = 0;
  get x() { return this._x; }
}

class B {
  instance private _x = 0;
  get x() { return this._x; }
}

const f = (a: A) => {}

// Should not be an error
f(new B());

🔍 Search Terms

instance private assignable

✅ Viability Checklist

My suggestion meets these guidelines:

⭐ Suggestion

Add an instance modifier that enforces that the member is only accessed with this.:

class A {
  instance private _x = 0;

  get x() {
    // fine
    return this._x;
  }

  getOtherX(o: A) {
    // error
    return o._x;
  }
}

📃 Motivating Example

Besides the common case using a class type as an interface, it's far too easy to end up in situations where users have multiple copies of a package and pass objects from one copy to another:

node_modules/foo/index.ts:

export class A {
  instance private _x = 0;
  get x() { return this._x; }
}
export const getX = (a: A) => a.x;

node_modules/bar/node_modules/foo/index.ts:

export class A {
  instance private _x = 0;
  get x() { return this._x; }
}
export const getX = (a: A) => a.x;

node_modules/bar/index.ts:

export {getX} from 'foo';

src/app.ts:

import {A} from 'foo';
import {getX} from 'bar';

// This causes a compile error but no runtime error:
getX(new A());

💻 Use Cases

Workarounds

There are some workarounds, but they are finicky and don't work completely.

Interface<T> helper:

You can define a helper to get just the instance interface of a class like:

type Interface<T> = {
  [K in keyof T]: T[K];
}

But using it isn't trivial:

export class AImpl {
  private _x = 0;
  get x() { return this._x; }
}
export const getX = (a: A) => a.x;

export type A = Interface<AImpl>;
export const A = AImpl;

Unfortunately, the exported type A doesn't fully represent the public interface of class AImpl. It doesn't have the static interface and it doesn't have the protected members which may be necessary for subclasses (and can still follow the rule that they are accessed only on the this instance).

/* @internal /

You can mark private members as /** @internal */ to exclude them from the published public types. This does not work for protected members though.

eliasm307 commented 1 year ago

+1

Its an issue I've been struggling with for a while now due to the setup of a project I'm working on (see details below).

I thought I was misunderstanding the "structurally typed" system for TS but I am now aware this is intentional behaviour (e.g. from this), however it would be good to get some examples to clarify what issues this is avoiding when the internal members are the same shape?

Maybe there are cases where knowing an instance comes from a particular parent via its internal members but since TS private/protected members are all public at runtime, its a bit heavy-handed for this to cause errors in all cases as some code is not written in such a way that the internal members coming from a particular parent class is a factor (especially if the internal members have an identical shape/type).

A way to opt out of this on a case-by-case basis would be good, however, I think a general compiler option would also be good to opt out of this and compare classes fully by structure only.

My Use Case

I have some packages where I need to symlink package A (e.g. a common dependency) into package B (e.g. an app) to be able to develop or test changes to package A as part of a real system.

Now let's say Package A and B both have another common dependency Package C and when the symlinking is done as above Package C gets duplicated so we have Package C in the node_modules for both Package A and B. In this case TS resolves Package C from different places and treats it as separate code (even if its exactly the same) and it shows compatibility issues for the private/protected members (which are exactly the same) in some parts of the code where Package A and B overlap using types from Package C.

Currently, I am having to add a lot of // @ts-expect-error comments to get around this so the code actually compiles and I can test the actual behaviour I'm interested in, which usually has nothing to do with Package C. Then when I'm done, I re-install the latest released Package C from the package registry and TS stops complaining then I have to remove the all the expect error comments. Then repeat again for the next change to Package C.

In some cases, Package C is something used in multiple variations of packages so this pain is amplified. Currently, I've built some logic into the build system to ignore some errors which can be reliably identified as being from this behaviour automatically but there are still unreliable cases where I have to manually add some expected error comments.

So TS is making this particular workflow awkward and the reason why is not clear to me.