Skip to content

Readonly properties type narrowing doesn't flow into inner function scopes #29281

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

Open
aoberoi opened this issue Jan 7, 2019 · 5 comments
Open
Labels
Domain: Control Flow The issue relates to control flow analysis Experience Enhancement Noncontroversial enhancements Suggestion An idea for TypeScript
Milestone

Comments

@aoberoi
Copy link

aoberoi commented Jan 7, 2019

TypeScript Version: 3.3.0-dev.20190105

Search Terms: Readonly, strictNullChecks, optional, inner function

Code

Using strictNullChecks: true:

interface Thing {
    foo?: (a: number) => boolean;
}

const t: Readonly<Thing> = {
    foo: (a) => true,
};

if (t.foo !== undefined) {
    const bar = () => {
        t.foo(5);
    }
}

Expected behavior:

👍 Compile

Actual behavior:

On the expression t.foo(5); the following error:

Cannot invoke an object which is possibly 'undefined'.
(property) foo?: ((a: number) => boolean) | undefined

Playground Link: https://www.typescriptlang.org/play/index.html#src=interface%20Thing%20%7B%0D%0A%20%20%20%20foo%3F%3A%20(a%3A%20number)%20%3D%3E%20boolean%3B%0D%0A%7D%0D%0A%0D%0Aconst%20t%3A%20Readonly%3CThing%3E%20%3D%20%7B%0D%0A%20%20%20%20foo%3A%20(a)%20%3D%3E%20true%2C%0D%0A%7D%3B%0D%0A%0D%0Aif%20(t.foo%20!%3D%3D%20undefined)%20%7B%0D%0A%20%20%20%20const%20bar%20%3D%20()%20%3D%3E%20%7B%0D%0A%20%20%20%20%20%20%20%20t.foo(5)%3B%0D%0A%20%20%20%20%7D%0D%0A%7D%0D%0A%0D%0A

Related Issues:

@Airblader
Copy link

Airblader commented Jan 7, 2019

This is different from #10927 because the const modifier here doesn't prevent the reassignment of t.foo, only the reassignment of t. So actually this is working as intended, IMHO.

Edit: ah, I missed that you made it readonly. Yeah, that makes it more interesting.

@aoberoi
Copy link
Author

aoberoi commented Jan 7, 2019

Doesn’t the Readonly<T> type mapping prevent the reassignment of t.foo?

If for some reason, any code in scope attempted to reassign that value, I’d expect the compiler to complain at that assignment, but not at the location of reading it.

@Airblader
Copy link

Yes, I had overlooked that and edited my answer right before you replied :-)

@weswigham weswigham added Experience Enhancement Noncontroversial enhancements Domain: Control Flow The issue relates to control flow analysis labels Jan 7, 2019
@RyanCavanaugh RyanCavanaugh added the Suggestion An idea for TypeScript label Mar 7, 2019
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Aug 16, 2019
@essenmitsosse
Copy link

Just in case someone (after 4 years) thought it might be due to the key being optional, here is another one:

const foo: { readonly a: string | number } = { a: '3' }
//             ^ This `readonly` is important

const outer = () => {
    if (typeof foo.a === 'string') { return }

    foo.a
    //  ^? (property) a: number

    const callback = () => 
        foo.a
    //      ^? (property) a: string | number
    //                             🫠 Y though?
}

Playground Link

I assume goes back to the old TS problem of readonly not meaning immutable — at least for object keys.

@upsuper
Copy link

upsuper commented Jul 17, 2024

I don't think you can really rely on the typing there. For example,

interface Thing {
    foo?: (a: number) => boolean;
}

const tMut: Thing =  {
    foo: (a) => true,
};

const t: Readonly<Thing> = tMut;

if (t.foo !== undefined) {
    const bar = () => {
        t.foo(5);
    };
    tMut.foo = undefined;
    bar();
}

This unsoundness would be accepted if what you described is allowed.

But I think a narrower case should be made accepted:

class Foo {
    constructor(private readonly elem: Element | undefined) {}
    foo() {
        if (!this.elem) {
            return;
        }
        setTimeout(() => {
            console.log(this.elem.textContent);
        });
    }
}

where the property is not only readonly, but also guaranteed not to be reassigned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Domain: Control Flow The issue relates to control flow analysis Experience Enhancement Noncontroversial enhancements Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

6 participants