microsoft / TypeScript

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

Object.freeze after object creation doesn't error #32253

Open karlhorky opened 5 years ago

karlhorky commented 5 years ago

TypeScript Version: 3.5.1

Search Terms: Object.freeze, freeze, freezing, properties, object, objects, property

Code

const a = Object.freeze({a: 0});
a.a = 1; // Error: Cannot assign read-only property

const b = {b: 0};
Object.freeze(b);
b.b = 1; // No error?

const c = {c: {d: 0}};
Object.freeze(c.c);
c.c.d = 1; // No error?

Expected behavior:

Cannot assign read-only property errors for the two lines with "No error?" above.

Actual behavior:

No errors

Playground Link: https://www.typescriptlang.org/play/?removeComments=true#code/MYewdgzgLgBAhjAvDA8gIwFYFNhQHQBmATllgF5YAUA3nAFwwAMAvgJQDcAUHHgsgIzsYAemEwAokSIgiDAMJwwYELDgQIASwDmYGCTgATALTgANgE8YAB2lWsRKOc6dQkWGiQxqaBiy7psXEIScio0Dk40PA8BIVEYADkQGHtpIgB+Z1doGGBPamAGagNfZmZ-TBx8YlIKSmA8YAiGhoNPQRExJJSpGXSgA

karlhorky commented 5 years ago

If this is an intentional design decision, it would be a good candidate for documentation on https://github.com/microsoft/TypeScript-Handbook/issues/1059

dragomirtitian commented 5 years ago

Typescript has no way to model the fact that a simple method call changes the type of its parameter.

The only way to model such changes are custom type guards, and these have to be used in some sort of conditional to work:

function freezeAndGuard<T>(p: T): p is Readonly<T> {
    Object.freeze(p);
    return true;
}

(function () {
    const b = {b: 0};
    if(!freezeAndGuard(b)) return;
    b.b = 1; // Err

    const c = {c: {d: 0}};
    if(!freezeAndGuard(c.c)) return;
    c.c.d = 1; // Err
})();

Playground link

karlhorky commented 5 years ago

Thanks for the answer! I suppose we can tag this as "Working as Intended" or similar and close it then?

In any case, good to have this documented at least here.

Maybe also good to add to the handbook, maybe one or multiple of the following:

karlhorky commented 5 years ago

Typescript has no way to model the fact that a simple method call changes the type of its parameter.

Would also be interested: is this a design limitation of TypeScript? Is there a possibility of changing it? It feels like it would not be a small change...

dragomirtitian commented 5 years ago

There were several proposals to implement something like custom type assertion ( #10421, #31512, #31879, #17760, #32130 just to name a few).

It feels small but it actually has big performance implications. From previous comment:

Typescript builds a graph of nodes that can impact the type of variables. This graph includes nodes of constructs that can impact the type of a variable (such as if, switch statemenets, assignments etc). With this change ALL calls would have to be included in this graph (the graph is constructed when type information about the function is not yet available, so there is no way to know that assertString has an impact on CF while other functions don't).

karlhorky commented 5 years ago

Thanks for the detailed explanation! Really helpful to have this all linked together 🙌

karlhorky commented 5 years ago

It feels small

I actually meant to say "not a small change", but I think the not got lost in the text. I've emphasized it.

jimmywarting commented 2 years ago

currently wish i could refactor this:

export class JSXIdentifier {
    readonly type: string;
    readonly name: string;
    constructor(name: string) {
        this.type = JSXSyntax.JSXIdentifier;
        this.name = name;
    }
}

into this:

export class JSXIdentifier {
    type: string;
    name: string;
    constructor(name: string) {
        this.type = JSXSyntax.JSXIdentifier;
        this.name = name;
        Object.freeze(this);
    }
}