typed-ember / glint

TypeScript powered tooling for Glimmer templates
https://typed-ember.gitbook.io/glint
MIT License
109 stars 51 forks source link

Issues with new modifier helper #570

Closed simonihmig closed 1 year ago

simonihmig commented 1 year ago

Updating Glint to the new stable version :tada: and removing my old any-based hacks due to the previously missing modifier-helper in this PR, I am seeing two issues come up here:

  1. Type instantiation is excessively deep and possibly infinite. - this seems to be related to my use of a conditional modifier here, that error does not pop up when not using this pattern
  2. Expected 1 arguments, but got 3. - this seems to be also an edge case, when applying the modifier helper on the imported on modifier (using @types/ember__modifier, have not tried native types yet). {{modifier "on" ...}} does not work due to https://github.com/emberjs/ember.js/issues/19869
dfreeman commented 1 year ago

For (1), do you have any examples that aren't using the on modifier? In general conditional modifiers like that should work, and I'm only seeing that excessively deep and possibly infinite message for cases where there are other type errors in play.

For (2), unfortunately I think we may be fundamentally unable to make on work well with {{modifier}}. The types for {{modifier}} and friends are fairly monstrous, and the way on's eventName argument is used to determine the Event subtype that the callback receives is more than TS's inference can handle—it ends up picking a (seemingly) arbitrary {{modifier}} signature that just doesn't work properly.

For the case you've linked there, have you considered using a single custom modifier to manage your event listeners? I suspect that would be easier to type, as well as avoiding runtime issues with https://github.com/emberjs/ember.js/issues/19869

simonihmig commented 1 year ago

For (1), do you have any examples that aren't using the on modifier?

No, it seems to come up only with on!

For the case you've linked there, have you considered using a single custom modifier to manage your event listeners

I just tried that, and indeed it works nicely and avoids all the mentioned problems! Thanks for the suggestion!

Gonna close this issue then, as it seems there isn't anything actionable left...