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

Readonly indexer and constructor usage: question #6781

Open
zhuravlikjb opened this issue Feb 1, 2016 · 13 comments
Open

Readonly indexer and constructor usage: question #6781

zhuravlikjb opened this issue Feb 1, 2016 · 13 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

@zhuravlikjb
Copy link

class A {
    readonly x;
    readonly [x: string]: string;

    constructor() {
        this.x = 5;  // that's okay
        this["a"] = "s"; // that yields an error
    }
}

Is that by design that readonly indexers for the same type in constructor yield errors?

@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label Feb 1, 2016
@RyanCavanaugh
Copy link
Member

I believe indexed assignments should be allowed in the constructor

@zhuravlikjb
Copy link
Author

It's also interesting how it should behave in case of inheritance.

class A {
    readonly x;
    readonly [x: string]: string;
}

class B {
   constructor() {
      super();
      this.x = 5;  // that's an error
      this["a"] = "s"; // should this be an error, too?
   }
}

@ahejlsberg
Copy link
Member

I don't think this is a bug. An index signature introduces no new properties and the constructor rule applies to properties. We could consider extending the rule to index signatures, but I'm not sure that's particularly useful. Is there are scenario that doesn't work with the existing rules?

Note that the following does work because this.x and this["x"] are just different ways of writing the same thing:

class A {
    readonly x: string;
    readonly [x: string]: string;

    constructor() {
        this.x = "s";     // Ok
        this["x"] = "s";  // Ok, same as this.x
        this["a"] = "s";  // Error, no declaration of a in this class
    }
}

@RyanCavanaugh
Copy link
Member

It seems like any case for disallowing this also applies to non-reaonly signatures. If you had a normal index signature, we'd let you write this["a"] = "s" in the constructor, of course, so the point that the a property is not declared is a red herring.

Now we've added readonly for both index signatures and properties and said that readonly doesn't apply in the constructor... but only for properties? That just seems weird.

@mhegazy mhegazy added Suggestion An idea for TypeScript In Discussion Not yet reached consensus and removed Bug A bug in TypeScript labels Feb 24, 2016
@RyanCavanaugh
Copy link
Member

@zhuravlikjb can you share a repro with more real-world names (maybe outline the broad usage of the class)? We talked about this for a while and couldn't agree on what the developer intent of a read-only index signature on a class is. Understanding how you're using this would be helpful.

@zhuravlikjb
Copy link
Author

@RyanCavanaugh Unfortunately, I develop a tool for TypeScript support, but written not in TypeScript, so this example is not a real-world thing, but just a test case, a theoretical question. If there are no real-user requests for this, you can freely close the issue.

@RyanCavanaugh RyanCavanaugh added Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature and removed In Discussion Not yet reached consensus labels May 10, 2016
@RyanCavanaugh
Copy link
Member

@zhuravlikjb thanks for the info. I'll leave this one awaiting feedback then.

@kaoDev
Copy link

kaoDev commented Jan 31, 2017

I just ran into this problem when I implemented a dictionary like class, the uscase is a table with items which contain an indexed value for each table column.

export class TableItem {
    readonly id: string; 
    readonly [key: string]: string;

    constructor(props: TableItem) {
        const {id, ...rest} = props;
        this.id = id;

        for(const key in rest) {
            this[key] = rest[key];
        }
    }
}

A workaraound-solution is to use the object.assign function in the constructor, but I think it would be better to have a consistent behavior of the compiler to allow indexed value assignments in the constructor.

This stackoverflow discussion is linked to this issue:
http://stackoverflow.com/questions/41953540/typescript-dictionary-with-readonly-keys/41953915#41953915

@axefrog
Copy link

axefrog commented Aug 22, 2017

My use case is that I implement a lot of immutable structures. I don't allow imperative mutation of key-value pairs, and instead construct new instances of things with the desired changes. This means that read-only indexers are a must, but that the set of keys being assigned is only known at runtime, and established within the constructor. Certainly I can work around this by having my properties be held internally and accessed indirectly via a readonly indexer, but then I'm changing the runtime characteristics of my implementation not because it's best for the application's architecture, but because TypeScript won't play ball. In this case my workaround involves casting to any first, but that's kind of hacky and prone to breakage when refactoring.

@slikts
Copy link

slikts commented Jul 19, 2018

My use case is constructing a frozen array-like:

class FrozenArraylike<A> {
  readonly length: number
  readonly [i: number]: A

  constructor(iterable: Iterable<A>) {
    let i = 0
    forEach(iterable[Symbol.iterator](), (value: A) => {
      this[i] = value
      i += 1
    })
    this.length = i
    Object.freeze(this)
  }
}

this[i] = value obviously doesn't work with the current rules, and I don't understand the reasoning:

An index signature introduces no new properties and the constructor rule applies to properties.

An index signature defines unspecific extra properties, and the constructor is the place to set them if they're readonly, because the object is created in the constructor so it's local mutation.

@tavianator
Copy link

Is there a workaround for this for numeric indices, other than using Object.defineProperty(this, i, { value: value })?

@slikts
Copy link

slikts commented Aug 16, 2019

I'd do this before the assignment:

// @ts-ignore: https://github.com/microsoft/TypeScript/issues/6781

The link being there to document why it's done.

@JoshuaWise
Copy link

Like @slikts, I also need this for a frozen array-like class. It was very surprising to me that this doesn't work.

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

9 participants