Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] 3.28 - Conditionally rendering a modifier throws error #20631

Open
fivetanley opened this issue Jan 25, 2024 · 0 comments
Open

[Bug] 3.28 - Conditionally rendering a modifier throws error #20631

fivetanley opened this issue Jan 25, 2024 · 0 comments

Comments

@fivetanley
Copy link
Member

fivetanley commented Jan 25, 2024

馃悶 Describe the Bug

Say we are rendering a template like this:

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

And a component definition like this:

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

export default class UsesOn extends Component {
  @tracked wasClicked = false;
  @tracked bindClickHandler = true;
  on = on;

  @action
  handleClick() {
    this.wasClicked = true;
  }

  @action
  toggleBindClickHandler() {
    this.bindClickHandler = !this.bindClickHandler;
  }
}

Calling toggleBindClickHandler to change the condition in the if statement should cleanly apply the on modifier. Instead, after this.bindClickHandler goes from true to false to true again, on the 3rd change when we go back to truth, we see the following error instead:

Uncaught (in promise) Error: You can only pass two positional arguments (event name and callback) to the `on` modifier, but you provided 4. Consider using the `fn` helper to provide additional arguments to the `on` callback.
    at OnModifierState.updateFromArgs (runtime.js:7277:1)
    at OnModifierManager.install (runtime.js:7486:1)
    at runtime.js:4840:1
    at track (validator.js:820:1)
    at TransactionImpl.commit (runtime.js:4839:1)
    at EnvironmentImpl.commit (runtime.js:4936:1)
    at inTransaction (runtime.js:4961:1)
    at Renderer._renderRoots (index.js:7779:1)
    at Renderer._renderRootsTransaction (index.js:7831:1)
    at Renderer._revalidate (index.js:7873:1)

馃敩 Minimal Reproduction

馃槙 Actual Behavior

Ember and/or glimmer seems to copy the arguments and append them to the existing arguments. This changes the amount of arguments from 2 to 4 and throws the error mentioned above:

Screenshot 2024-01-24 at 21 13 54

馃 Expected Behavior

The modifier should be able to conditionally removed/enabled without error.

馃實 Environment

  • Ember: - 3.28.12
  • Node.js/npm: - 16/8.19.4
  • OS: - Mac OS
  • Browser: - Safari/Firefox/Chrome

Additional Context

Does not seem to happen on modern versions of Ember: https://limber.glimdown.com/edit?c=MQAgMgpgLg5AziKE5RAKDQWhAYwPYB2AJgJZQmECGANiALZ6kBmJEATliAA6Vvk3UAniEpcu1VkXqMSLdpwbNWbEThzI4EKUzZ46IEgRSUC6zHHxcIGNAAN7AcwBWCCQDcI3NhDesA7mgkdFx4fCAAwnohBBAEqDp6IDAAAg4SdHTsAPT4wYSxsADcgXlhAN6IbJQ4ANZaIAC%2BIAn6KWlBmWxZUFW1hg4wxUEh5SCEIghQeFyQHtQA8gSNzbqtyRB0AEbZirLKgxgQAB4jqEQQTJQArtSoONSUcAgAEhDU1HgA6qHUUsdIxAQkTyMTiIDKaBAIGSPWqdSk%2BCuYIAvCAAAzFSFjJaoqYzHxvRaYqGGHDeTIokAACgAlCBkQA%2BRAACxIcAAdIiwQBqVEARmKUIc0AMcAAoh4CLTwViod4oFc2EsoKyOVzUABSEAAJnpyNRGKxDQwUIAPEhgg8kAzZSBTVwGQBNPBXEDMygeXASWr1FWeTZXKBTJZlMoqtmcl1xBpNciZDmmrIOk1Qu0BoPjUN42aEpYAInuJFqeZZEdJ5IKMYZ4W9NUT6eDNttpobhFtUND1NkpY5bIlsWpuzkKnDHPGMELtRgPfZ5Y2BRpdJjtptqdTiyEXqLNUom2onj8zIHBKW7fBZWA3dHs-FkuXa7XVMoO5ABDwfhpZ9DWVk99T9cDRssUTC1xEoa00GNexbDQIA&format=glimdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant