Skip to content

if condition type does not correctly consider mutable type narrowing #897

@NullVoxPopuli

Description

@NullVoxPopuli

Reported internally at AB by @mmun

For example, we're all familiar with this situations, which pushes us to use const variable declarations:

let x = true;

fn(() => {
  x // may not be true
  
})

Here is an example of what can go wrong: https://share.glimdown.com/9etSk_K0TiGDiIm5mtYgrg

And here is how it looks in raw TS (to eliminate any potential ember weirdness, we simulate the behavior here).

In particular, we have this situation:

class MyComponent {
  args: { data: true }
  constructor(args: { data: true; }) {
    this.args = args;
  }
  /* ... */
}

function render() {
  if (state.data) {
    return new MyComponent({ 
      get data() {
      //  ^^^^
      //  Type 'boolean' is not assignable to type 'true'. (2322)
        return state.data;
      }
    })
  }

  return;
}

which accurately describes the same behavior we see in Ember:

{{#if state.data}}
  <MyComponent @data={{state.data}} />
{{/if}}

Because accessing state.data is essentially a getter (due to the lazy nature of arguments in a signals-based component framework), we don't actually have a guarantee that state.data is true.

So what should happen?

Like how TS recommends that we change let to const, and errors due to the variable being able to change, so too should we error if MyComponent's @DaTaistrue`. We can't narrow on mutable properties, because we don't know if they'll change.

Why is this a problem?

This is contrived, but there was an expectation internally on a complex object for it to exist.

Most simply, we can reproduce the issue via:

  handleClick = async () => {
    state.data = false;

    if (!this.args.data) {
      throw new Error('Uh oh, this.args.data is falsey');
      // ^ this is expected, of course, because
      //   @data === state.data, always, due to lazy accessing
    }
  }

An option:

  • don't mutate data outside your control (a correct thing to do anyway) -- this component doesn't own state.data, so it should not touch it.

What should Glint do?

  • explore changing the if condition transform (template to TS) to detect property access, and widen the type -- this may mean going from true => boolean, or T to T | null | undefined (this may not be possible to do well -- idk, needs research)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions