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

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

Open
karlhorky opened this issue Jul 4, 2019 · 8 comments
Open

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

karlhorky opened this issue Jul 4, 2019 · 8 comments
Labels
Docs The issue relates to how you learn TypeScript PursuitFellowship Help wanted from Pursuit fellowship; others please avoid until Dec 19

Comments

@karlhorky
Copy link
Contributor

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
Copy link
Contributor Author

If this is an intentional design decision, it would be a good candidate for documentation on microsoft/TypeScript-Handbook#1059

@dragomirtitian
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor Author

karlhorky commented Jul 4, 2019

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
Copy link
Contributor

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
Copy link
Contributor Author

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

@karlhorky
Copy link
Contributor Author

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.

@RyanCavanaugh RyanCavanaugh added the Docs The issue relates to how you learn TypeScript label Jul 8, 2019
@sandersn sandersn added the PursuitFellowship Help wanted from Pursuit fellowship; others please avoid until Dec 19 label Sep 18, 2020
@jimmywarting
Copy link
Contributor

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);
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docs The issue relates to how you learn TypeScript PursuitFellowship Help wanted from Pursuit fellowship; others please avoid until Dec 19
Projects
None yet
Development

No branches or pull requests

5 participants