component helper w/ class constructor returns nullable value

[simonihmig]

In a component I yield another (private) component with the component helper, like this:

{{yield (hash field=(component this.FieldComponent data=this.internalData))}}

this.FieldComponent is the class (constructor) of the other yielded component. It should only be used in the yielded form, and will not be exposed in the app re-exports, that's why I don't pass a string (like in (component "path/to/field")).

The signature for the component is (simplified):

{
  Args: {...};
  Blocks: {
    default: [
      {
        field: WithBoundArgs<typeof FieldComponent, 'data'>;
      }
    ];
  };

Now Glint complains about this:

It seems to me the issue is that it thinks that the return value of the component helper could be null (which is not the case!), while the block params in the signature don't allow this obviously.

Having had a quick look at the type definitions for the component helper, and with my non-hardcore-expert-level TS skills, I still interpret this definition in such a way, that passing a valid component constructor (which I do) could still return null or undefined. Which is not true, right? Wouldn't it be more correct to say that when the component argument is a valid constructor, then the return value is a PartiallyAppliedComponent, otherwise in case of null or undefined being passed in returns just that?

[simonihmig]

Although the definition here seems right. Is that supposed to get picked up by TS instead of the other? But even then, why is it thinking it can be null? 🤔

[dfreeman]

Wouldn't it be more correct to say that when the component argument is a valid constructor, then the return value is a PartiallyAppliedComponent, otherwise in case of null or undefined being passed in returns just that?

That's exactly what the signature above the one you linked does 🙂 I think the type tests for {{component}} would probably have caught this if it were a general all-the-time problem (see e.g. these assertions), so I wonder if there's something different about your types 🤔

Just to confirm, this.FieldComponent isn't nullable, is it?

[simonihmig]

Just to confirm, this.FieldComponent isn't nullable, is it?

No, it isn't:

I think the type tests for {{component}} would probably have caught this if it were a general all-the-time problem (see e.g. these assertions), so I wonder if there's something different about your types 🤔

Yeah, makes sense! 🤔

What I am working on is open source, I can push my work and send you a link later, if that helps!

[simonihmig]

@dfreeman I have now pushed my work in progress here, if you want to take a look: https://github.com/CrowdStrike/ember-headless-form/pull/14

I silenced the Glint error mentioned above here. The component constructor is assigned here, without being able to be null.

[dfreeman]

@simonihmig I just pulled main in that repo (since it looks like your PR was merged) and as far as I can tell everything is typechecking as expected now—did you figure out what was happening?

[simonihmig]

Sorry, I should have provided a more stable reproduction, than just pointing to a PR...

Things there have changed quite a bit since raising this issue, so the Glint error reported here IIRC did go away when introducing ensure-safe-component (which had its own issue #518) and the type juggling added to workaround that issue.

I just tried to create a stable reproduction based on the earlier commits. Although I was able to reproduce again, I saw that there was a flaw in how I handled my own generic types. And I was able to fix that error by a simple type cast. So it seems you are right that this wasn't a "general all-the-time problem". Still not understanding really where the null type came from that was reported in the error message...

I could push my current reproduction (without that type fix), but at this point I guess we don't want to waste more time on this? Therefore closing this issue. Sorry for taking your time here!

[dfreeman]

Sorry for taking your time here!

No problem at all! Sorry it took me so long to take a look at this that everything had shifted around in the meantime 😄

I could push my current reproduction (without that type fix), but at this point I guess we don't want to waste more time on this?

If you're satisfied with how you've got things working currently, then I'm happy to leave this closed. But if you do run into the issue again elsewhere (or, for anyone else who comes along and finds this issue: if you run into it), I'm happy to reopen this and dive deeper.