镜像站点 · 本页由第三方 GitHub 只读镜像提供，非 GitHub 官方站点，不接受任何登录或凭据输入。前往 github.com
Skip to content

Control flow analysis of aliased conditions is not able to narrow object properties #46412

Closed
Closed
Control flow analysis of aliased conditions is not able to narrow object properties#46412
Labels
Design LimitationConstraints of the existing architecture prevent this from being fixed

Description

@DavidZidar
David Zidar (DavidZidar)
opened on Oct 18, 2021

Suggestion

🔍 Search Terms

  • Control flow
  • Aliased conditions
  • Type narrowing

✅ 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

TypeScript 4.4 added "Control Flow Analysis of Aliased Conditions and Discriminants" which is a great feature, but unfortunately it seems unable to narrow the types on object properties which limits the benefit.

📃 Motivating Example

Take null checks for instance, the following code results in a compiler warning in the AliasedControlFlow-method. The RegularControlFlow method works fine even though it does the exakt same comparison.

interface ITest {
    prop?: string;
}

function AliasedControlFlow(test: ITest) {
    const hasProp = test.prop != null;

    // Object is possibly 'undefined'.
    return hasProp ? test.prop.big() : "";
}

function RegularControlFlow(test: ITest) {
    return test.prop != null ? test.prop.big() : "";
}

💻 Use Cases

This would really help when converting code that is currently not using strictNullChecks as null checks may already be aliased in existing code.

Activity

  1. ahejlsberg commented on Oct 18, 2021

    @ahejlsberg
    Anders Hejlsberg (ahejlsberg)
    on Oct 18, 2021
    Member

    The issue here is that prop is not a readonly property. From #44730, the PR that implemented the feature:

    Narrowing through indirect references occurs only when the conditional expression or discriminant property access is declared in a const variable declaration with no type annotation, and the reference being narrowed is a const variable, a readonly property, or a parameter for which there are no assignments in the function body.

  2. Anders Hejlsberg (ahejlsberg)
    added
    Design LimitationConstraints of the existing architecture prevent this from being fixed
    on Oct 18, 2021

  3. DavidZidar commented on Oct 18, 2021

    @DavidZidar
    David Zidar (DavidZidar)
    on Oct 18, 2021
    Author

    Anders Hejlsberg (@ahejlsberg) Thank you for the response! In this case readonly can not be used as it is an object.

    'readonly' type modifier is only permitted on array and tuple literal types.

    I'm not sure I understand the last part of the paragraph, there are no assignments in the function body except for the alias.

    Is this a use case that is intended to be fixed in the future or is the limitation too much work to resolve?

  4. DavidZidar commented on Oct 18, 2021

    @DavidZidar
    David Zidar (DavidZidar)
    on Oct 18, 2021
    Author

    The language keyword readonly did not work but the Readonly<T> mapped type does work, it may take care of some of the use cases as long as all properties on the type can be readonly.

  5. MartinJohns commented on Oct 18, 2021

    @MartinJohns
    Martin Johns (MartinJohns)
    on Oct 18, 2021
    Contributor

    The property must be marked readonly:

    interface ITest {
    readonly prop?: string;
}

  6. DavidZidar commented on Oct 18, 2021

    @DavidZidar
    David Zidar (DavidZidar)
    on Oct 18, 2021
    Author

    Martin Johns (@MartinJohns) Right, I understand, that's what Readonly<T> does when used on the function argument.

  7. ahejlsberg commented on Oct 18, 2021

    @ahejlsberg
    Anders Hejlsberg (ahejlsberg)
    on Oct 18, 2021
    Member

    Is this a use case that is intended to be fixed in the future or is the limitation too much work to resolve?

    It's basically a cost/benefit trade-off in control flow analysis. In order to support mutable properties we'd have to check that there are no assignments to a property between the declaration of the aliased condition that references the property and the check of that aliased condition. It's possible to do so (anything is possible), but it is non-trivial and it's not clear the added complexity and potential performance cost is worth it.

  8. DavidZidar commented on Oct 18, 2021

    @DavidZidar
    David Zidar (DavidZidar)
    on Oct 18, 2021
    Author

    Anders Hejlsberg (@ahejlsberg) I understand, that sounds reasonable. There could be a lot more code between the alias and the usage.

    Do I close this issue now or leave it open for further discussion?

  9. Ryan Cavanaugh (RyanCavanaugh)
    closed this as completedon Feb 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Design LimitationConstraints of the existing architecture prevent this from being fixed

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions