microsoft / TypeScript

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

SUGGESTION: add support for writeonly properties on interfaces #21759

Open Stevenic opened 6 years ago

Stevenic commented 6 years ago

I'd like to resurrect an old discussion around the desire to have getters/setters support for interfaces: #11878

Obviously we can use readonly to express a property on an interface that has just a getter but there's no way of expressing that a property should have just a setter. In issue #11878 there was an idea proposed to add the concept of a writeonly property designed to cover this need but the consensus was that there wasn't enough real-world scenarios to justify such a feature. So let me try and add one.

We have a situation where we have a child object that we want to publish data to a parent but while we want the parent to know about its child we don't want the child to know about its parent. We've ruled out the use of events because we only want a single subscriber and we need to return a Promise to the child to let them know when we're done. Instead we've opted to establish what we would have called a "weak reference" between the child and parent back in the days of COM. The interface in TypeScript looks something like this:

interface Adapter {
     onDataReceived: (data: any) => Promise<void>;
     publishData(data: any): Promise<void>;
}

As you can see data flows bidirectionally between the parent and child and while we've received a couple of questions about why the interface is the way it is, it's generally easy enough to grok from a TypeScript perspective.

The issue we just ran into, however, is that a developer on our team just created a class in ES6 that implements this interface and the result ended up being.... yuck :(

If we literally implement this interface in a declarative way in ES6 it looks something like:

export class WebAdapter {
     get onDataReceived() {
          return this.callback;
     }
     set onDataReceived(cb) {
          this.callback = cb;
     }
     postData(data) {
     }
}

Not only is it crappy that you have to define a getter and a setter, the fact of the matter is we're never going to ask for the callback back so the getter is pointless here. So what did our dev do? He did this:

export class WebAdapter {
     onDataReceived(data) {
          // will be replaced by parent
     }
     postData(data) {
     }
}

That technically works and what's nice is you have some sense of the signature of the handler but it makes my skin crawl to look at it. If I was to mirror that in my TypeScript interface you'd have zero clue that onDataReceived() was something I expect you to override. What I really want the developer to have to write implementation wise is this:

export class WebAdapter {
     set onDataReceived(cb) {
          this.callback = cb;
     }
     postData(data) {
     }
}

That's the proper contract for a weak reference but I have no way of expressing it in TypeScript. While it's very rare that you need to do this it doesn't make it any less valid a scenario. The addition of "writeonly" properties would give me a way to express this.

ghost commented 6 years ago

I can see the use of this as it would allow you to do this without indirection:

interface I {
    // User of the interface can register a callback, but not call it.
    writeonly callback: () => void;
}
class C implements I {
    // Only I can call this.
    callback: () => void;
}

However, in your situation this.callback is presumably a private member, written to in set onDataReceived. In which case, why not just have a onDataReceived(cb) method? Having a method would allow registering multiple callbacks instead of writing over the current one.

Stevenic commented 6 years ago

I didn't use a function because I specifically didn't want multiple listeners. The reason for that is that when a publisher (the child) publishes to its parent, the parent returns a Promise that the child can await on to know when the parent has completed processing the request. In our specific use case the child will typically be holding open an HTTP request and waiting for the parent to successfully finish processing the request before ACK'ing to the calling server. If I used a function for this now I have to deal with the potential of multiple subscribers and that will never be the case.

To me... This is a classic parent-child case where the parent wants a strong reference on the child and the child needs to maintain a weak reference to its parent. Classically you would potentially pass the parent into the constructor of the child. But in our case the Parent isn't responsible for factoring the child, nor do we want the child to have to know the parents type.

I've been talking somewhat abstractly but if we want to get concrete this is the specific interface in our SDK that I'm referring too:

https://github.com/Microsoft/botbuilder-js/blob/master/libraries/botbuilder/src/activityAdapter.ts

I'm happy to drill into more specifics and definitely open to suggestions.

ghost commented 6 years ago

Regardless of the behavior you want it to have, requiring classes to implement set onDataReceived as a setter doesn't seem to accomplish more than requiring setOnDataReceived as a method would. A property that consists only of a setter is basically just a method with one parameter.

samchon commented 6 years ago

writeonly statement

Consideration

Since TypeScript v2.0 update, defining abstract method getter and setter are possible. Defining getter in interface level, it's also possible by the readonly statement. However, defining setter in interface level is not.

abstract class AbsClass
{
    public abstract get x(): number;
    public abstract set y(val: number);
}

interface ISolution
{
    readonly x: number;
    writeonly y: number;
}

function some_function(obj: AbsClass | ISolution): void
{
    // y = x^2 + 4x + 4
    obj.y = Math.pow(obj.x, 2) + 4 * obj.x + 4;
}

If developer wants to restrict a member to have only setter (write only), then the developer must define an abstract class, who takes unneccessary dependency. Such dependency can be solved by the writeonly statement.

Also, within framework of the Least Privilege Policy, some_function needs only reading on member x, writing on member y. The some_function doesn't need to write x or read y. Following the policy is possible by implementing the writeonly statement.

In My Case

I've published a TypeScript-STL project, who migrates C++ STL to TypeScript. I agree to this suggestion and I'll be glad if such writeonly feature comes into the TypeScript. With the writeonly statement in interface level, I can make my library to be much stronger.

namespace std
{
    // BASIC INTERFACES
    export interface IForwardIterator<T>
    {
        readonly value: T;
        next(): IForwardIterator<T>;
    }
    export interface IOutputIterator<T>
    {
        writeonly value: T;
        next(): IOutputIterator<T>;
    }

    // WRITE-ONLY STATEMENT IS THE BEST SOLUTION
    export function copy<T, 
            InputIterator extends IForwardIterator<T>, // READ-ONLY
            OutputIterator extends IOutputIterator<T>> // WRITE-ONLY
        (first: InputIterator, last: InputIterator, output: OutputIterator): OutputIterator
    {
        for (; std.not_equal_to(first, last); first = first.next())
        {
            output.value = first.value;
            output = output.next();
        }
        return output;
    }
}

STL follows Iterator Pattern and the Iterator Pattern is used very a lot in the <algorithm> functions. Iterators are parameterized and some of them are readonly, some of them are writeonly, and some of them are both.

For example, std.copy() is a representative <algorithm> function requires the writeonly statement. Without the writeonly statement, the first alternative solution is to defining the OutputIterator to extends an abstract class who takes the Unneccessary Dependency.

Iterator.value Readable Writable
std.vector.iterator O O
std.deque.iterator O O
std.list.iterator O O
std.set.iterator O X
std.map.iterator O X
std.insert_iterator X O
std.front_insert_iterator X O
std.back_insert_iterator X O

The 2nd alternative solution is to defininig the OutputIterator's type to be general interface allowing both reading & writing. However, the solution causes incompleteness error-detecting in compile level.

The OutputIterator (IGeneralIterator) doesn't guarantee the parameterized Iterator.value is writable. Even std.set.iterator, who allows only reading, can be assigned onto the IGeneralIterator. It causes not compile-error but run-time error, which violates main purpose of the TypeScript.

namespace std
{
    export interface IForwardIterator<T>
    {
        readonly value: T;
        next(): IForwardIterator<T>;
    }
    export interface IGeneralIterator<T>
    {
        value: T;
        next(): IGeneralIterator<T>;
    }

    // WITHOUT WRITE-ONLY, IT CAUSES INCOMPLETENESS ON COMPILE LEVEL
    export function copy<T, 
            InputIterator extends IForwardIterator<T>, // READ-ONLY
            OutputIterator extends IGeneralIterator<T>> // DANGEROUS
        (first: InputIterator, last: InputIterator, output: OutputIterator): OutputIterator
    {
        for (; std.not_equal_to(first, last); first = first.next())
        {
            output.value = first.value;
            output = output.next();
        }
        return output;
    }
}

let v: std.vector<number> = new std.vector(4, Math.random()); // GENERAL
let s: std.set<number> = new std.set(); // SET.ITERATOR.VALUE IS READ-ONLY
s.push(1, 2, 3, 4);

//----
// NO ERROR ON COMPILE
//----
// BE RUNTIME ERROR
// IT VIOLATES MAIN PURPOSE OF TYPESCRIPT
std.copy(v.begin(), v.end(), s.begin());
ghost commented 6 years ago

@samchon That's a better use-case as it involves using the same type in both readonly and writeonly ways.

interface I {
    x: number;
}
function copy(input: Readonly<I>, output: Writeonly<I>): void {
    output.x = input.x;
}
samchon commented 6 years ago

@andy-ms Thanks for replying. Writeonlyinterface, it sonds interesting. Is it planned to be implemented?

Anyway, unlike readonly and writeonly statement who can specify restriction on member variables level, Readonly and Writeonly interfaces restrict on object level. Even my example, members on ISolution require different types of restriction. The iterators, only value members are required to be restricted.

ghost commented 6 years ago

Nobody's currently assigned to this issue, so don't expect it to be implemented in the near future.

boris-fabernovel commented 6 years ago

@andy-ms : now you really prove us right that typescript is not a superset of javascript anymore, but is slowly but surely becoming another language

DrSensor commented 6 years ago

Found another use case here with getter and setter have different data type.

interface OutgoingRequest {
  readonly requestBody: string | null
  writeonly requestBody: string | object
}
dead-claudia commented 5 years ago

BTW, this could solve React's ref invariance issue.

interface ReactRef<T> {
    current: T | void
}

interface ReactRefConsume<T> {
    writeonly current: T
}

interface Attributes {
    ref: ReactRefConsume<HTMLElement>
}

export function createRef<T>(): ReactRef<T>
wandyezj commented 4 years ago

I have also encountered this issue.

It seems as if TypeScript interfaces should be able to fully describe any JavaScript object. This is NOT the case today with properties that can only be set on classes.

class C {
    constructor(private value: boolean = false) {
    }

    set property(value: boolean) {
        this.value = value;
    }

    print() {
        console.log(this.value)
    }
}

interface I {
    // Suggested Solution: writeonly property: boolean
    property: boolean;
    print: () => void;
}

const e: I = new C();

// Should be allowed
e.property = true;

// Should NOT be allowed (returns undefined) but it looks like it is.
console.log(e.property)

e.print();

Are there reasons why implementation of this feature would be difficult or why it wouldn't be a good idea?

bunnybones1 commented 4 years ago

+1 Supporting writeonly is probably ideal, but at the minimum, getting a property that only implements a setter should throw an error: https://github.com/microsoft/TypeScript/issues/37689

Artazor commented 3 years ago

Looks like it could be a reasonable semi-measure that will allow somehow model a covarince/contravariance (that I suppose will never be implemented in TS)

type AnyFunction = (...args: writeonly never[]) => unknown

It's kinda dangerous, yet looks better than

type AnyFunction = (...args: any[]) => unknown

However, for the full support it should be properly integrated with mapped types. something like this:

type InverseVariance<T> = {
       writeonly [P in readonly T]: T[P];
       readonly [P in writeonly T]: T[P];
}

Just fantasying...

@DanielRosenwasser @Igorbek @isiahmeadows ?

lgarron commented 3 years ago

I'm currently working on a library where this would be really useful. We essentially need to represent async properties. It's very ergonomic to support setters, but async getters would be really unintuitive. So I want to support just setters for now, without associated getters. We have ≈ a dozen properties so far, and expect to have more, so I would really like to be able to set them programmatically. Here's a reduce example of what I want to do:

class Prop<T> {
  set(value: T | Promise<T>) { /* */ }
  get(): Promise<T> { /* this may involve async processing/validation/sanitization */ }
}

class TwistyPlayerModel {
  alg: Prop<Alg> = /* ... */;
  puzzle: Prop<string> = /* ... */;
}

class TwistyPlayer extends HTMLElement {
  model = new TwistyPlayerModel();
  writeonly alg: Alg;
  writeonly puzzle: string;
  constructor() {
    super();
    for (const [name, prop] of this.model) {
      Object.defineProperty(this, name, {
        set: (v) => prop.set(v)
      })
    }
  }
}

(See https://github.com/cubing/cubing.js/blob/381fba0859c20387131d2afa5225898c46da829c/src/cubing/twisty/dom/twisty-player-model/props/TwistyPlayerModel.ts#L80-L80 for the real TwistyPlayerModel.)

Without writeonly, the only "safe" options I see is to write out each getter without an associated setter. This bloats the code (both the source and the bundled size), and results in a lot of boilerplate to maintain. The example above has only one writeonly line per property, which is quite compact compared to any alternatives I can think of.

But I think it's actually easier to maintain type-safe code the way I wrote above. I can't find of a satisfactory way of tricking the type system into accepting this right now, but I think writeonly would make this pretty straightforward.

thw0rted commented 2 years ago

I think this could be addressed by #43662 so I just want to make sure anybody subscribed here also wanders over there at some point. Basically, what we want is set x(n: number) {...}; get x(): undefined {} (or maybe never?). I don't think we need to have a list of compelling use-cases as long as my arm for this to make sense. What we get today is, if you have set x(n: number) {...}; and code tries to read foo.x, it types as number but is in fact undefined at runtime -- objectively wrong, unsafe behavior by the typechecker.

simeyla commented 2 years ago

Would make my day if a missing getter in Typescript was automatically transpiled to return the 'raw' value originally set via the setter. It would be much less surprising to get your original value back out than 'undefined, and would double as a really nice language feature to allow setters to have side-effects without messy boilerplate.

set name(value: string)
{
    console.log('name changed to ' + value);
}

Could be transpiled to something like:

set name(value: string)
{
   _name = value;   
    console.log('name changed to ' + value);
}
get name() {
    return this._name;
}
private _name: string;

Edit: I realize this isn't going to happen. Just that it would be nice.

thw0rted commented 2 years ago

That's not going to happen, as it a) is the opposite of what native JS classes do in this case, and b) violates the Design Goals (and non-goals).

I'd like to see the property treated as writeonly; failing that, get yourself a linter and set up the accessor-pairs rule. At least you'll be forced to write out some kind of getter any time you make a setter, and your code will behave consistently.

upsuper commented 9 months ago

I think this could be addressed by #43662 so I just want to make sure anybody subscribed here also wanders over there at some point.

It's not fully addressed by that in two ways:

  1. You need mapped types for a generic utility type, but you can't define getter and setter on a mapped type. (See https://github.com/microsoft/TypeScript/issues/51890#issuecomment-1358098791)
  2. It still doesn't prevent reading and checking against null values, even if the return type is never.

So I think it would still be good to have writeonly properties even when we can already have different types between getter and setter.

d07RiV commented 6 months ago

This would be a big step towards supporting contravariant interfaces. A common use case is React refs - the current field is supposed to be write-only when passed to a component, but Typescript doesn't support that and all refs are incorrectly treated as covariant.

RyanCavanaugh commented 6 months ago

What shortcomings are there with using this?

interface Foo {
    get foo(): never;
    set foo(x: string);
}
jsejcksn commented 6 months ago

What shortcomings are there with using this?

interface Foo {
    get foo(): never;
    set foo(x: string);
}

It doesn't accurately describe the data structure, and the following are syntax noise compared to a simple writeonly modifier:

RyanCavanaugh commented 6 months ago

It doesn't accurately describe the data structure

How so?

jsejcksn commented 6 months ago

How so?

Data descriptors and accessor descriptors aren't identical, even though they might appear indistinguishable to consumers.

Such discrimination might potentially provide value if TypeScript were to support this in the future — for example — as an implementation constraint that allows a plain writable data descriptor, but not a setter accessor descriptor (maybe to prevent possible side-effects in an implementation). Imaginary syntax example:

type WriteonlyFoo = { writeonly foo: string };

// Allowed
const o: WriteonlyFoo = Object.defineProperty({}, "foo", { writable: true });

// Not allowed
const o: WriteonlyFoo = Object.defineProperty({}, "foo", {
    set(value: string) { /*...*/ },
});
RyanCavanaugh commented 6 months ago

If it's a data field, how is it "write-only" ?

jsejcksn commented 6 months ago

If it's a data field, how is it "write-only" ?

@RyanCavanaugh I'm not sure what you're asking. In any case, the syntax noise is the bigger pain at present.

snarbies commented 6 months ago

If it's a data field, how is it "write-only" ?

Because that's the intended contract. Or because it improves reasoning about variance. Or because it provides parity with accessors. Same way a property can be protected, even though there's really not such a thing. That said, I don't know to what degree write-only is actually enforced. You can certainly write to a property that only defines a setter without any complaints from the type checker.

phaux commented 6 months ago

What shortcomings are there with using this?

interface Foo {
    get foo(): never;
    set foo(x: string);
}

It doesn't make Foo contravariant in regards to its foo property.

type WriteOnlyRef<in T> = {
    set current(current: T)
    get current(): never
}

function setRefToCanvas(ref: WriteOnlyRef<HTMLCanvasElement>) {
    ref.current = document.createElement("canvas")
}

type Ref<T> = { current: T }

const ref: Ref<HTMLElement | null> = { current: null }

setRefToCanvas(ref)
Argument of type 'Ref<HTMLElement | null>' is not assignable to parameter of type 'WriteOnlyRef<HTMLCanvasElement>'.
  Types of property 'current' are incompatible.
    Type 'HTMLElement | null' is not assignable to type 'never'.
      Type 'null' is not assignable to type 'never'.

Playground

sorgloomer commented 5 months ago

What shortcomings are there with using this?

interface Foo {
    get foo(): never;
    set foo(x: string);
}

For me, it is the lack of mapped type support, i.e.:

type Writeonly<T> = { set [K in keyof T](value: T[K]) }; // error, cannot declare get or set types in mapped types

If we had a built-in Writeonly type, that would suffice, I could use it to cobble together any type I wish.

CraigMacomber commented 2 months ago

What shortcomings are there with using this?

interface Foo {
    get foo(): never;
    set foo(x: string);
}

For me, it is the lack of mapped type support, i.e.:

type Writeonly<T> = { set [K in keyof T](value: T[K]) }; // error, cannot declare get or set types in mapped types

If we had a built-in Writeonly type, that would suffice, I could use it to cobble together any type I wish.

If thats the only issue, then maybe this thread can be considered a duplicate of https://github.com/microsoft/TypeScript/issues/43826 which tracks the specific limitation of mapped types not allowing control of setters (and includes a suggestion to solve it with "writeonly" as one of the the 5 ways I propose to solve it)