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

Allow Object.defineProperty() to definitely assign properties #42919

Open
5 tasks done
brainkim opened this issue Feb 23, 2021 · 6 comments
Open
5 tasks done

Allow Object.defineProperty() to definitely assign properties #42919

brainkim opened this issue Feb 23, 2021 · 6 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

@brainkim
Copy link

brainkim commented Feb 23, 2021

Suggestion

✅ Viability Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.

⭐ Suggestion

It would be nice if Object.defineProperty() and Object.defineProperties() calls could be read via the constructor to definitely assign class properties.

📃 Motivating Example

https://www.typescriptlang.org/play?ssl=12&ssc=2&pln=1&pc=1#code/MYGwhgzhAEDiBOBTRAXR9oG8BQ0-QDswBbRALmghXgEsCBzAbl33qVTvoqtoef2jAA9gR4BXYCiHwAFEVLdqnAJRYWAvCgAWNCADp5iaAF5CJRPw14A8gCMAVokl6AJogBmdRAAV4QgA7oKACeMtq6ADTQAERsyCic0VE4Vlb0qDKqKak5SChi8ATQAAZaiCAgQtAAJJiGAL7F6jnQ9c0C9cqWeG31QA

class Greeter {
    name: string;
    greeting: string;
    constructor(name: string) {
        this.name = name;
        Object.defineProperty(this, "greeting", {
            get() {
                return `hello ${name}`
            }
        });
    }
}

Currently the greeting property in the class will cause the error Property 'greeting' has no initializer and is not definitely assigned in the constructor.

💻 Use Cases

This is not a big deal because we can use a “definite assignment assertion“ (!) after the greeting property identifier, but it would be nice for TypeScript to be aware of these defineProperty calls.

🔍 Search Terms

constructor, Object.defineProperty, Object.defineProperties, definitely assigned, strictPropertyInitialization, definite assignment operator
Property has no initializer and is not definitely assigned in the constructor.

This is potentially a duplicate of the following issues:

Feel free to close as duplicate!

@RyanCavanaugh RyanCavanaugh added 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 labels Feb 23, 2021
@RyanCavanaugh
Copy link
Member

Why do you use an instance getter instead of a prototype getter? Enumerability?

@brainkim
Copy link
Author

brainkim commented Feb 23, 2021

@RyanCavanaugh One reason to define a getter property in the constructor is if you want to lazily cache a property for first access. You could make it a prototype getter, but you’d have to store the cached value somewhere. This might mean a WeakMap, which is nice but cumbersome, or it might mean an additional property on the object, which can mess with deep object equality testing and complicate the object’s representation in the console and in memory. The nice thing about using Object.defineProperty() in the constructor is that you can hide the cached value in the constructor’s local scope, and never have to worry about it being exposed on the object itself.

class Parent {
  constructor() {
    /* assigning regular properties */
    let children: Array<Children> | undefined;
    Object.defineProperty(this, "children", {
      get() {
        if (!children) {
          children = createChildArray(this.firstChild);
        }
        return children;
      }
    });
  }
}

This sort of pattern also allows for advanced memory patterns. For instance, you could conceivably clear the cached value in a microtask or macrotask callback directly within the constructor, or use a fancy new WeakRef, so that the value is only cached for repeated synchronous accesses and released later so that the actual object which is retained can be kept compact and normalized. I don’t actually recommend doing any of this; it’s only for memory-obsessive JavaScript developers like myself, and if you’re writing getters which return non-primitives or are otherwise costly it’s often better just to make them methods to signify the cost of the operation to callers. Nevertheless, it’s a potential technique to have in your toolbelt.

MDN also recommends self-overwriting getters as a solution to cached getters, but I’m not sure if TypeScript supports deleting prototype getters (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/get#smart_self-overwriting_lazy_getters).

That being said, this issue isn’t just concerning getters but any Object.defineProperty() usage in the constructor. For instance, the following will also error.

class Greeter {
    name: string;
    greeting: string;
    constructor(name: string) {
        this.name = name;
        Object.defineProperty(this, "greeting", {
            value: `hello, ${name}`,
            writable: false,
            configurable: false,
        });
    }
}

@ThaJay
Copy link

ThaJay commented Mar 4, 2021

I run into the same problem with this much simpler code:

class Thing {
  someString: string;
  someNumber: number;

  constructor (data) {
    this.setData(data);
  }

  setData(data) {
    this.someString = data.someString;
    this.someNumber = data.someNumber;
  }
};

Now I can use ! or I can duplicate the logic inside of setData, but it seems a bit naive of the ts compiler to not follow a call to a function on this before it decides the code is noncompliant.

@sandersn
Copy link
Member

Note that the compiler has basic support for this in JS, but not TS. (I'm not sure the OP code works, though.)

@MichaelTheriot
Copy link

MichaelTheriot commented Jan 6, 2022

This suggestion only speaks of class properties but it would also be desirable for regular objects.

Consider this callable object with regular assignment:

image

This correctly produces:

const typeA: {
    (): number;
    x: number;
}

If I want x to be not configurable, not enumerable, and not writable then I have to use defineProperty:

image

But the x property is now missing on the type. I would expect instead to get:

const typeB: {
    (): number;
    readonly x: number;
}

I don't know if there needs to be a separate issue created for supporting regular objects.

@Pyrolistical
Copy link

Pyrolistical commented Feb 12, 2022

I ran into this issue and have 2 workarounds. Apologies for writing this is js + jsdocs, but these workarounds also work in just ts.

The problem I ran into was trying to add properties to an array. Typescript will error with ts2339 if you attempt set it directly, so you need to use Object.defineProperty. But then typescript doesn't believe you have set the property on the array and errors.

First workaround do a forced cast. Cast to known then cast to your desired type.

Second workaround is to use assertion function

/**
 * @typedef {{yada: number}} Yada
 * @typedef {string[] & Yada} ArrayWithYada
 */

/**
 * @return {ArrayWithYada}
 */
function createThing() {
    const result = ['hello']
    Object.defineProperty(result, 'yada', {
        value: 42
    })
    return result // this errors
}

/**
 * @return {ArrayWithYada}
 */
function createThingCastWorkaround() {
    const result = ['hello']
    Object.defineProperty(result, 'yada', {
        value: 42
    })
    return /** @type {ArrayWithYada} */ (/** @type {unknown} */(result))
}

/**
 * @return {asserts input is Yada}
 */
function assertYada(input) {
  // fake assertion
}
/**
 * @return {ArrayWithYada}
 */
function createThingAssertionWorkaround() {
    const result = ['hello']
    Object.defineProperty(result, 'yada', {
        value: 42
    })
    assertYada(result)
    return result
}

playground

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

6 participants