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

Suggestion: read-only modifier #12

Closed
RyanCavanaugh opened this issue Jul 15, 2014 · 132 comments
Closed

Suggestion: read-only modifier #12

RyanCavanaugh opened this issue Jul 15, 2014 · 132 comments
Assignees
Labels
Committed The team has roadmapped this issue Fixed A PR has been merged for this issue Suggestion An idea for TypeScript

Comments

@RyanCavanaugh
Copy link
Member

Some properties in JavaScript are actually read-only, i.e. writes to them either fail silently or cause an exception. These should be modelable in TypeScript.

Previous attempts to design this have run into problems. A brief exploration:

interface Point {
    x: number;
    y: number;
}
interface ImmutablePoint {
    readonly x: number;
    readonly y: number;
}
var pt: ImmutablePoint = { x: 4, y: 5 }; // OK, can convert mutable to non-mutable
pt.x = 5; // Error, 'pt.x' is not a valid target of assignment

var pt2: Point = pt; // Error, cannot convert readonly 'x' to mutable 'x'

// Possibly bad behavior
var pt3: Point = { x: 1, y: 1 };
var pt4: ImmutablePoint = pt3; // OK
pt3.x = 5; // pt4.x is also changed?

// Really bad behavior
/** This function was written in TypeScript 1.0 **/
function magnitudeSquared(v: { x: number; y: number }) {
   return v.x * v.x + v.y * v.y;
}
// Now try to use it with ImmutablePoint
console.log(magnitudeSquared(pt)); // Error, cannot use readonly object in non-readonly call

Possible solutions?

  • Allow bivariance of mutability: this is very unsound
  • Something else clever? C++ did not do well with const contamination
@everson
Copy link

everson commented Jul 21, 2014

I think this is probably the most important feature in terms of making Typescript a safer language. E.g. You are guaranteed that the object will not be mutated along the way and that simplifies reasoning a lot.

however, I don't see the problem with magnitudeSquared, it does not change 'v', so, it should work fine with both read-only and mutable objects.

@basarat
Copy link
Contributor

basarat commented Jul 30, 2014

👍

@RyanCavanaugh
Copy link
Member Author

I don't see the problem with magnitudeSquared, it does not change 'v',

The problem is, you don't know this without reading the function implementation, and you might not have access to the implementation. Let's say someone gives you their library with a 1.0 TypeScript definition file:

// Docs say: calculates the squared magnitude of a vector
declare function magnitudeSquared(v: { x: number; y: number }): number;
// Docs say: rescales a vector to have length 1 and returns
// the scale factor used to normalize the vector
declare function normalize(v: { x: number; y: number }): number;

One of these functions modifies the object, one of them doesn't. You can't tell by looking at their signatures -- they're identical.

Now we have two obvious options

Assume all functions arguments are actually treated as read-only?

That fails to catch this problem

var p: ImmutablePoint = { x: 1, y: 3 };
var pN = normalize(p); // Failure to catch immutability violation

Worse, now you have to opt in to non-readonliness. When you upgrade your definition file to have the read/write information, now you're going to have to write mutable basically everywhere:

// I have to write the word 'mutable' this many times?
declare function normalize(v: { mutable x: number; mutable y: number }): number;
declare function invert(v: { mutable x: number; mutable y: number, mutable z: number }):void;
declare function changeRotation(v: { mutable theta: number; mutable phi: number }): void;

Assume all function arguments are actually mutable?

Now I can't use magnitudeSquared without a cast:

var p: ImmutablePoint = { x: 1, y: 3 };
var len = magnitudeSquared(p); // Error, this function might modify p!

And you get to write readonly everywhere:

// I have to write the word 'readonly' this many times?
declare function magnitudeSquared(v: { readonly x: number; readonly y: number }): number;
declare function length(v: { readonly x: number; readonly y: number, readonly z: number }): number;
declare function dotproduct(v: { readonly theta: number; readonly phi: number }): number;

@joewood
Copy link

joewood commented Nov 4, 2014

Can this be re-opened as a request? Given the increased use of functional programming with JavaScript and immutable data structures (see React and Flux). I think this would be great as an opt-in feature, maybe with a top-level decorator enabling it to avoid the issues of default behavior (i.e. ignore the check by default).

@RyanCavanaugh
Copy link
Member Author

I'm not sure what you're asking? The issue is currently open.

@joewood
Copy link

joewood commented Nov 4, 2014

Sorry, misread the status.

@Jxck
Copy link

Jxck commented Nov 19, 2014

+1 for readonly keyword support.

@IanYates
Copy link

+1 for something along these lines. The problems described by @RyanCavanaugh are quite real but the suggestion by @joewood to have some sort of decorator seems to be a smart one. I guess it might be along the lines of 'use strict' at the start of functions but probably more explicit, like
mutable class vs immutable class
and
mutable module vs immutable module
to specify the defaults within a block of code at the module or class level. For back-compat reasons, mutability would have to be assumed, right?

Perhaps along the same lines, where ? is used on a parameter to indicate it's optional, could other annotations be added, along with one indicating either (like how any throws away types)?

@RandScullard
Copy link

"The perfect is the enemy of the good." As TypeScript stands today, the compiler won't even tell me when I set a property with no setter -- with the full implementation available. This has been raised as an issue multiple times, and those other issues all point back to this one, so it seems pretty important that progress be made in this area. To me, having the readonly decorator and catching the assignment-with-no-setter error would be a huge step in the right direction, even if it doesn't solve the third-party library problem.

Edit: Removed reference to readonly decorator. Readonly is a tar pit, and my point is around get/set, not interfaces.

@danpaul88
Copy link

+1 for a readonly modifier or similar. The scenario I am frequently running into is wanting to define an interface with properties that only requires a getter, which I would then expect the compiler to enforce by generating an error if the code tries to set a readonly property. Implementations would be free to provide a setter if they choose to do so, but it should only be usable in code if the object is being referenced as the implementing class or a derivative of it.

My personal preference would be the way c# does it, allowing you to specify get and/or set in the interface, but the readonly case is by far the most useful (other than the full get/set case).

@Eyas
Copy link
Contributor

Eyas commented Nov 27, 2014

Thinking out loud here:

While readonly fields are important, perhaps an easier thing to implement, which doesn't have some of the problems @RyanCavanaugh outlined is immutable classes/interfaces.

Suppose you have:

immutable interface ImmutablePoint {
    x: number;
    y: number;
}

Given the properties of an imutable object, we can change how assignment behaves, to instead clone and freeze in JS.

So that:

var pt3: Point = { x: 1, y: 1 };
var pt4: ImmutablePoint = pt3; // OK -- pt3 is copied
pt3.x = 5; // pt4.x -- does not change

The pt4 assignment would basically look like:

...
// compiled JavaScript
var pt4 = pt3.clone(); // your favorite clone() scheme
pt4.freeze();
...

The compiler can then optimize the clone & freeze into only a freeze if:
1- we are using a literal assignment
2- possibly, if the variable/value we are copying is dead after this line (when dealing with global scope ,etc.)

Some problems remain with this:

  1. Unclear what to do about deep properties; I'm tempted to say "nothing" here... the immutability guarantee might be seen as a guarantee to not change references to other objects (values of pointers), rather than values within these objects.
  2. For the magnitudeSquared problem, we'd need to declare it as magnitudeSquared(v: immutable v { x: number, y: number }).

@joewood
Copy link

joewood commented Nov 29, 2014

I was actually thinking of something more like the const modifier in C++. This would be applied to the object instance, providing an immutable view of an existing object. So something more like:

interface Foo {
    x : number;
    getConstBar() const : const Bar;
    getBar() : Bar;
}

function doNotMutate( obj : const Foo ) {
   obj.x = 3; // !! compiler error
   var bar = obj.getBar(); // !! compiler error, cannot call a non-const function 
   var constBar = obj.getConstBar(); // OK, can invoke a const function now have a const Bar
}

function mutate( obj: Foo ) {
    obj.x = 3; // works
    var bar = obj.getBar();  // works too
}

This way it's an opt-in on the interface, and can be passed down to child objects through accessor functions (or getters) that are declared const. Declaring a function const essentially applies the modifier to this (again, in the same way as C++ works).

So, in practical terms for libraries like react.js, the setState function would be mutable but the accessor functions to state and props would be immutable.

@metaweta
Copy link

I prefer Eyas' suggestion. It's clean and simple. JavaScript has so many ways to modify state: adding/removing/deleting properties, side-effecting getters, proxies, etc. that introducing a concept like const will necessarily be horribly complicated.

There's also the issue of claiming that a method or argument in a JS library is const when it turns out not to be; there's no way to check it and not even runtime errors will be generated unless the compiler wraps arguments in a read-only proxy.

@Eyas
Copy link
Contributor

Eyas commented Dec 14, 2014

Perhaps even more explicit, another suggestion similar to my previous one:

No clone & freeze, simply, immutable {...} can only be assigned by types that are:

  1. Structurally compatible, and
  2. Also immutable.

A toImmutable() on every object converting object to immutable object is created. Which basically has clone and freeze symantics. Thus:

var pt3: Point = { x: 1, y: 1 };
var pt4: ImmutablePoint = pt3; // NOT OK -- Point and ImmutablePoint are incompatible
var pt5: ImmutablePoint = pt3.toImmutable(); // OK, structurally compatible
pt3.x = 5; // pt4.x -- does not change

The call .toImmutable(); basically implements clone and freeze.

The type definition for freeze() could also be augmented.. not sure how yet.

@Lenne231
Copy link

Lenne231 commented Mar 6, 2015

👍 I really like Eyas' suggestion, but the .toImmutable(); function should not be part of TypeScript.

@Lenne231
Copy link

Lenne231 commented Mar 7, 2015

I've summarized some of my thoughts on immutables in TypeScript. May be this could be help.

// Primitives
val x = 7, y = 'Hello World', z = true; // ok -> all primitives are immutable by default

// Objects 
val p1 = { x: 120, y: 60 }; // ok -> all attributes are immutables

p1.x = 45; // error -> p1 is immutable
p1['y'] = 350; // error -> p1 is immutable

var p2 = { x: 46, y: 20 };
p1 = p2; // error -> p1 is immutable

val keyValue = { key: 'position', value: p2 }; // error -> p2 is not immutable

// Arrays
val numbers = [0, 1, 2, 4]; // ok -> all elements are immutable

numbers.push(9); // error -> numbers is immutable

val points = [p1, p2]; // error -> p2 is not immutable

// Functions
type Point2D = { x : number, y : number };

function multiply(s : number, val point : Point2D) {
    return val { x: s * point.x, y: s * point.y };
}

multiply(2, { x: 32, y: 20 }); // ok -> { x: 32, y: 20 } is immutable

multiply(2, p1); // ok -> p1 is immutable

multiply(3, p2); // error -> p2 is not immutable

function multiply2(s : number, val point : Point2D) : val Point2D {
    return { x: s * point.x, y: s * point.y };
}

// Type Assertion
val p3 = <val Point2D>p2;

// Classes
class Circle {

    private x: number;
    private y: number;
    private val r: number;

    constructor(x : number, y : number, r : number) {
        this.x = x;
        this.y = y;
        this.r = r; // ok -> in constructor
    }

    setRadius(r: number) {
        this.r = r; // error -> this.r is immutable
    }
}

var circle = new Circle(100, 200, 20);
circle.x = 150; // ok -> circle.x is not immutable

class Circle2 {
    constructor(private val x : number, private val y : number, private val r : number) { }
}

@gustavderdrache
Copy link

I am very strongly in favor of a conservative approach here. Probably the simplest would be allowing const for properties in declarations:

interface Point {
    const x: number;
    const y: number;
}

var origin: Point = {
    x: 0,
    y: 0,
}

The semantics of const here would be the same as if it were a binding: no assignments are allowed. Nothing else would be implied (the C++ const semantics would get hairy pretty quickly).

That is, the following is allowed:

interface Foo {
    const map: { [key: string]: string };
}


var x: Foo = { map: {} };
x.map['asdf'] = 'hjkl';

The compiler should assume all properties are mutable unless told otherwise. This sucks for people who like to use objects in a read-only fashion (myself included), but it tracks the underlying JavaScript better.

I can think of at least four kinds of immutability in JS:

  1. const bindings, as offered by ES6,
  2. Get-only computed properties,
  3. Ordinary properties configured with writable: false in their descriptors, and
  4. Objects passed through Object.freeze.

From a type perspective, numbers 2 and 3 act the same, but the machinery is technically different. I see some comments above about annotating an entire object as immutable, which wanders into the territory of point 4, but I think that opens a can of worms. (More below.)

I thought about giving a rough spec, but here's a fun question:

Given:

interface Foo {
  const x: number;
}

interface Bar {
   x: number;
}

var x: Foo =  { x: 1 },
    y: Bar = { x: 1 };

Is the following assignment compatible?

var z: Foo = y;

That is, can we allow mutable properties to "downgrade" into immutable variations? It makes sense in function scenarios:

function showPoint(point: { const x: number; const y: number }): string {
    return "(" + point.x + ", " + point.y + ")";
}

console.log(showPoint(x), showPoint(y));

While (rather) confusing, it does have the advantage (?) of jiving with what JavaScript offers with get-only properties:

var x = 0;
var obj = {
    get x() {
        return x++;
    }
};

console.log(obj.x, obj.x);

Despite the question above, I would suggest behavior like the following: the presence of const in a property declaration is treated by the compiler as a read-only value; any and all assignments are banned. In the presence of const properties, assignments are compatible if the compiler can verify that the property is readable:

interface Foo {
    const x: number;
}

var x: Foo = { x: 1 }; // okay
var y: Foo = { get x(): { return 1; } }; // also okay, because 'x' is readable
var z: Foo = { set x(_x): { return; } }; // no, 'x' has no getter

Compilation output for a class would be roughly as follows:

class Foo {
    const x = 1;
}
var Foo = (function () {
    function Foo() {
        Object.defineProperty(this, "x", {
            value: 1,
            enumerable: true,
            configurable: true,
            writable: false,
        });
    }
    return Foo;
})();

Not sure how it would compile for a module. Perhaps it would use Object.defineProperty on the module object or instead try to align with ES6?

Declaring a function as const would have the possibly counterintuitive effect of making the function declaration immutable, not its return value.

That is, given the following:

interface Math {
    const sin(x: number): number;
}

Any instance of Math can't have its sine implementation replaced. (But users could say (<any> Math).sin = /* ... */ if they wanted.)

This can cause issues with declaration merging, though.

interface Foo {
    const func(): void;
}

interface Foo {
    func(x: number): void;
}

In the case above, have we a) implicitly added a setter for the property func, or b) removed immutability? If the meaning is a, then we're okay. If it's b, then we should error, because the second interface has discarded a previously-existing guarantee.

My gut says we should go with interpretation b, given the intent we're trying to express. And if that's the case, then perhaps there should be interface syntax for getter and setter properties so that the following won't be an error:

interface Foo {
    get x(): number;
}

interface Foo {
    set x(_x: number): void;
}

As opposed to above, the intent here is not immutability but the allowed operations on a property.

As an aside, I'm on the fence about allowing const on indexers:

interface Whitelist<T> {
    const [key: string]: T;
}

Perhaps it's best to leave it off for now and let it come into existence if people ask for it.

With all of this in hand, the notion of an immutable object seems easy: add some keyword (perhaps frozen to match the underlying Object.freeze, or immutable for obvious reasons) that means "all properties on this object are const.

But then you have the unenviable position of figuring out if a method is safe to call on an immutable interface:

immutable interface Vector {
    x: number;
    y: number;
    magnitude(): number; // safe
    normalize(): void; // not safe
}

It's not really enough to look at void return types, though:

var x = {
    x: number;
    y: number;
    debug() {
        console.debug("vector: ", this.x, ",", this.y);
    }
};

It's clearly safe to call x.debug() even if it were of an immutable type because no property is modified. But I shudder to think of what its return type should look like. const void?

So to make a long reply short (too late), we could add const as a modifier to property declarations to indicate that the compiler should prevent the user from mutating them. It's difficult to make other guarantees.

@Lenne231
Copy link

There is also an EcmaScript 7 proposal on immutable data structures: https://github.com/sebmarkbage/ecmascript-immutable-data-structures

@Eyas
Copy link
Contributor

Eyas commented Mar 12, 2015

To add to that, keep in mind that const exists in ES6. We should try to avoid overloading keywords that are going to have a specific meaning in future ES standards.

@gustavderdrache
Copy link

I'm not sure the immutable data structures proposal solves the problem of how to annotate that some properties can't be modified. They're two separate issues.

(I'm not married to the const usage I invented. It was mostly to get the highlighter to work.)

@basarat
Copy link
Contributor

basarat commented Mar 12, 2015

I guess with first class immutable data structures the type system will need some way to signify immutability . This will allow the compiler to do compile time checks.

@GameDaddy
Copy link

excellent!

@zpdDG4gta8XKpMCd
Copy link

zpdDG4gta8XKpMCd commented Dec 17, 2016

An important missing side note: #13003 (comment)

readonly prevents direct mutation of the declared property through that reference. That's it.

In plain English it means that #13002 is a use case yet to be looked at

@NN---
Copy link

NN--- commented May 20, 2017

I didn't find anywhere best practices for readonly instead of get-only properties.
Should I change all get properties to readonly ones?
The main difference that I see is that readonly property will become a regular read-write property in JS whereas get-only property is not.

@joelday
Copy link
Contributor

joelday commented May 20, 2017

@NN--- Generally the best practices are: private readonly members and public and protected get-only properties on classes. Use readonly for interfaces that will be implemented by classes that have get-only properties. Can add examples later. OTOH maybe StackOverflow, especially their new docs site is a better venue.

@NN---
Copy link

NN--- commented May 20, 2017

@joelday TypeScript misses shorthand for such get-only properties.
If only I could write 'public get prop: string;' instead of readonly + get.

@mhegazy
Copy link
Contributor

mhegazy commented May 20, 2017

A getter only property will be inferred as readonly automatically.

@NN---
Copy link

NN--- commented May 20, 2017

@mhegazy I understand this.
I meant the short syntax as in C# and other languages:

interface A {
    readonly x: number;
}

class B implements A {
    readonly _x: number;
    get x(): number {return this._x; }
}

class C implements A {
    get x(): number; // This should be equivalent to B.x implementation
}

@mhegazy
Copy link
Contributor

mhegazy commented May 20, 2017

And readonly x: number is not good enough?

@mhegazy
Copy link
Contributor

mhegazy commented May 20, 2017

Or u r looking for public get private set?

@NN---
Copy link

NN--- commented May 20, 2017

It is debatable and as I said I didn't find what is considered to be a good practice.
The main concern is that readonly becomes 'read-write' in the output JS and it can be changed by some bad code.
@joelday Suggests to use get-only properties in the class though.
@mhegazy Yes, but be more specific , constructor only set.

@kitsonk
Copy link
Contributor

kitsonk commented May 20, 2017

Terse get syntax like C# was discussed in #10911.

Since properties (at the moment) aren't ES spec, it could be argued though that the readonly modified in a class could make a non-writable value descriptor. If someone wanted to truly shadow the value, a getter only would be the other way. So this:

class A {
  readonly a = 'foo';
  get b() {
    return 'bar';
  }
}

Would de-sugar to:

var A = (function () {
    function A() {
    }
    Object.defineProperty(A.prototype, "a", {
        value: 'foo',
        writable: false,
        enumerable: true,
        configurable: true
    });
    Object.defineProperty(A.prototype, "b", {
        get: function () {
            return 'bar';
        },
        enumerable: true,
        configurable: true
    });
    return A;
}());

@mhegazy
Copy link
Contributor

mhegazy commented May 20, 2017

Why is readonly diffrent from private? Privacy is not maintained in the generated js either.

@NN---
Copy link

NN--- commented May 20, 2017

@mhegazy You are correct. I mistakenly thought that private members are implemented in some different way.
Then it makes sense for same way readonly implementation.

I guess it is offtopic now. It needs some other syntax for truly readonly properties to be implemented by defineProperty and privates via symbol or some other nice hack.
Thanks.

@paleo
Copy link

paleo commented Sep 25, 2017

The usage of readonly is not completely clear for me. I wrote a question on Stackoverflow.

This following code compiles, but is it semantically correct?

interface Counter {
  readonly name: string
  readonly value: number
  inc(): number
}

function counterFactory(name: string): Counter {
  let value = 0
  return {
    get name() {
      return name
    },
    get value() {
      return value
    },
    inc() {
      return ++value
    }
  }
}

The member value is declared as readonly but its value is changing.

I would like to know if this use of readonly on the member value is a good way to proceed. The syntax is OK. But is this example semantically correct in TypeScript? Is that what the modifier readonly is for?

@NN---
Copy link

NN--- commented Sep 25, 2017

@paleo readonly is similar to const, it doesn't define deep immutability, just some way to make less mistakes.

const x = {a:1};
x = {a:2}; // Error , cannot change x
x.a = 2; // Ok, const is not deep
let x: {readonly a:number;} = {a:1};
x = {a:2}; // x is mutable do whatever you want
x.a = 2; // Error, 'a' is readonly and doesn't allow reassignment.

For true immutability, you need to write code which makes types immutable.

@raveclassic
Copy link

@paleo readonly doesn't guarantee that a value won't change in time, it just doesn't let you write to it. That's why the word is readonly, not constant.

@zpdDG4gta8XKpMCd
Copy link

it just doesn't let you write to it

that's not true either: #13002

@raveclassic
Copy link

@Aleksey-Bykov Previous issue title was a bit closer then...

@paleo
Copy link

paleo commented Sep 26, 2017

I quote @ahejlsberg from his PR:

A good way to think of it is:

  • readonly = cannot be modified by me
  • const = cannot be modified, period

In my question on StackOverflow, I wrote two use cases using readonly. The first case matches well to "cannot be modified by me". But the second case, that I have reported above, would rather be a "cannot be modified by you".

Is that what the modifier readonly is for? Is it valid from a semantic point of view?

@kitsonk
Copy link
Contributor

kitsonk commented Sep 26, 2017

Is that what the modifier readonly is for? Is it valid from a semantic point of view?

Like a lot of the TypeScript typing system, it is not enforceable at run-time, it is a type contract. const is a ECMAScript run-time concept which TypeScript can fully model the run-time behaviour. readonly correctly aligns to run-time behaviour when the property in question has a getter only or is a value property that is not writable. At other times, it is a design time only construct to improve type safety (just as string or number are design time only constructs).

@paleo
Copy link

paleo commented Sep 27, 2017

readonly correctly aligns to run-time behaviour when the property in question has a getter only or is a value property that is not writable.

Thank you, it's clearer.

I realize that I already had the answer in 2016, from @RyanCavanaugh . At that time the ES5 getter solution was not available to me because my code had to remain ES3 compatible, so I left angry and didn't take the time to understand the information. He said:

You can already have a getter-only wrapper around a private variable, which achieves the same semantics at the cost of slightly increased class size.

This is the final answer: yes, the readonly modifier can be used with an ES5 getter to achieve this semantics.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Committed The team has roadmapped this issue Fixed A PR has been merged for this issue Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests