[Bug] The `on` modifier does not work with the `modifier` helper

[lolmaus]

🐞 Describe the Bug

This works:

<div {{(if this.condition (modifier 'my-modifier' this.bar))}}>

🔬 Minimal Reproduction

This doesn't:

<div {{(if this.condition (modifier 'on' 'scroll' this.bar))}}>

Can't create a Twiddle because Twiddle is severely outdated, sorry.

😕 Actual Behavior

Uncaught Error: Assertion Failed: Attempted to invoke (-resolve "modifier:on"), but on was not a valid modifier name.

🤔 Expected Behavior

Should apply the on modifier to the element.

🌍 Environment

• Ember: 3.28.4
• Node.js/npm: 14.16.0
• OS: Build: Ubuntu, Browser: Windows 10
• Browser: Chrome, Firefox

CC @simonihmig

[Windvis]

I did some testing and it's not only the on modifier that has this problem. It seems that all helpers and modifiers that come from Glimmer don't work with the contextual helpers.

The error is thrown from the -resolve helper which gets wrapped around the string by a template transform plugin.

The built-in modifiers and helpers don't seem to be registered in the container which is why the -resolve helper throws that error. It is possible to work around this in app code by importing the helper or modifier and manually registering it.

For example:

import { getOwner } from '@ember/application';
import { on } from '@ember/modifier';

export default class ApplicationRoute extends Route {
  constructor() {
    super(...arguments);
    getOwner(this).register('modifier:on', on);
  }
}

ember-source contains the setupEngineRegistry util that already registers some built in components, so adding all the needed registrations there would be a potential fix for this problem. The question is, is that the correct fix or are there better alternatives?

[Windvis]

I found the following in the RFC:

Some built-in helpers or modifiers may not be resolvable with the helper and modifier helpers. For example, (helper "debugger") and (helper "yield") will not work, as they are considered keywords. For implementation simplicity, we propose to forbid resolving built-in helpers, components and modifiers this way across the board (i.e. a runtime error). We acknowledge that there are good use cases for this feature such as currying the array and hash helpers, and will consider enabling them in the future on a case-by-case basis.

So it seems like these are not supported by design and it isn't actually a bug? In that case a more clear error message would still be nice though.

Does that mean we need an RFC for this?

[chancancode]

@Windvis I think that sections are referring to cases that are really "keywords" that aren't really helpers, debugger and yield are good examples, pretty sure (debugger) and (yield). {{mount}} and {{outlet}} also comes to mind.

But I am pretty sure (modifier "on") is supposed to work, the RFC had a bunch of examples showing just that. So I think this is an implementation issue (i.e. a bug). I'll confirm tomorrow.

[chancancode]

Presumably on isn't the only case? Are there other built-in things that doesn't work?

[chancancode]

Yes, this should work. In addition to fixing this bug we should also do a pass on all the other helpers/modifiers/keywords/RFCs we have to see if there is anything else missing, but they don't have to block each other.

[yads]

A use case for this is to conditionally add a click handler to an element e.g.

<div role="{{if this.doButton 'button'}}" {{(if this.doButton (modifier "on" "click" this.handleClick))}}>
  Click Me
</div>

Currently my workaround is to have a modifier that adds an event listener conditionally

export default modifier(function conditionalOn(
  element,
  [condition, event, handler],
  { capture, once, passive }
) {
  if (condition) {
    element.addEventListener(event, handler, { capture, once, passive });
  }
});

[fivetanley]

For future travelers: As a workaround, here's what I ended up doing (thank you @NullVoxPopuli for helping me find the import):

// app/components/my-component.js

import Component from '@glimmer/component';
import { on } from '@ember/modifier';
import { tracked } from '@glimmer/tracking';
import { action } from '@ember/object';

export default class MyComponent extends Component {
  on = on;
  @tracked bindClickHandler = true;
  @action handleClick() {
    this.
  }
}

then my template looks like

<button
  {{(if this.bindClickHandler (modifier this.on "click" this.handleClick))}}
>Click me
</button>