ember.js icon indicating copy to clipboard operation
ember.js copied to clipboard

[Bug] {{on}} modifier on input fired before bound value is updated

Open olyckne opened this issue 4 years ago • 4 comments

🐞 Describe the Bug

When using the {{on}} modifier on an input the callback is fired before the bound value is updated. From the documentation <Input @id="input-name" @value={{this.name}} {{on "input" this.validateName}} />

In validateName I would assume I can validate the name by using this.name but it doesn't have the correct value at this time. This differ from the "old" way of using an argument for the event: <Input @id="input-name" @value={{this.name}} @input={{this.validateName}} />

This might be indented but it wasn't clear for me, especially coming from using the "old" way.

🔬 Minimal Reproduction

https://ember-twiddle.com/89e1a8168304360b322981218ee218ff

😕 Actual Behavior

When using the {{on}} modifier the bound value isn't updated before the modifier callback is run, which differs from the "old" way using an event argument. Example: If I have <Input @value={{this.name}} {{on "input" this.validateName}} /> and I type "John Doe" in this input field, by the time this.validateName is called this.name will have the value "John Do". Using the "old" way <Input @value={{this.name}} @input={{this.validateName}} /> this.name will have the value "John Doe".

🤔 Expected Behavior

When using the bound value in the modifier callback I expect it to be the updated value. If this is intended maybe it should at least be clarified in the documentation? Example: If I have <Input @value={{this.name}} {{on "input" this.validateName}} /> and I type "John Doe" in this input field. by the time this.validateName is called I expect this.name to have the value "John Doe".

🌍 Environment

  • Ember: 3.22
  • Node.js/npm: node v12.19.0, npm 6.14.8
  • OS: MacOS 10.15.7 (19H2)
  • Browser: Safari Version 14.0 (15610.1.28.1.9, 15610), Chrome 86.0.4240.111, Firefox 82.0

olyckne avatar Oct 22 '20 13:10 olyckne

I investigated a related issue in https://github.com/emberjs/ember-test-helpers/issues/1128#issuecomment-1049685543. The problem is that {{on}} does not schedule the event listener onto the run loop, specifically the actions queue, unlike @input, which is a classic component action.

When you wrap your listener in schedule('actions', () => ...), it works as expected. I've updated your Twiddle to show that.

buschtoens avatar Feb 24 '22 11:02 buschtoens

When you wrap your listener in schedule('actions', () => ...), it works as expected. I've updated your Twiddle to show that.

@buschtoens hm, the bug is still present for me in your updated Twiddle.

using next(() => ...) seems to fix it though.

olyckne avatar Feb 24 '22 15:02 olyckne

Interesting! I'm using Firefox and there it works. You're using Chrome, I guess? There it's indeed not working.

next(() => ...) guarantees that the callback is only executed in the next cycle of the event loop, at which point all event listeners and queues of the previous run loop frame are definitely guaranteed to have all been called.

This comes at a cost though: it's essentially calling setTimeout(() => ...), meaning that the call will not resolve synchronously. Consequentially you won't be able to synchronously wait for the call to have occurred nor get its return value in the invoking context.

I wanted to file a very closely related {{on}} modifier bug later anyway — In fact it might even be the same bug. I'll keep an eye out for the Chrome/Firefox discrepancy.

buschtoens avatar Feb 24 '22 17:02 buschtoens

@buschtoens Yeah, I tried in Safari and Chrome.

olyckne avatar Feb 28 '22 08:02 olyckne

I've stumbled across this bug, while refactoring from the {{input helper to the Octane <Input

tehmaestro avatar Oct 25 '22 14:10 tehmaestro

Any update on this bug?

rgaiacs avatar Sep 22 '23 09:09 rgaiacs

The bug relies on the super old two-way binding behavior, and I think we're wanting to encourage people to not use <Input>, and instead use native <input>

In this example, I don't think I was able to reproduce the problem.

NullVoxPopuli avatar Sep 22 '23 09:09 NullVoxPopuli

Dear @NullVoxPopuli, Thanks for the fast reply.

Is a PR to https://guides.emberjs.com/release/components/built-in-components/ welcome to high-light this?

rgaiacs avatar Sep 22 '23 10:09 rgaiacs

absolutely! that'd be wonderful!

NullVoxPopuli avatar Sep 22 '23 10:09 NullVoxPopuli