emberjs / ember.js

Ember.js - A JavaScript framework for creating ambitious web applications
https://emberjs.com
MIT License
22.46k stars 4.21k forks source link

Racing condition when action placed on back burner fires when element removed from DOM #15770

Closed RyanNerd closed 6 years ago

RyanNerd commented 6 years ago

Here's the twiddle: https://ember-twiddle.com/2a82465b6be428c43c7e86290c5e9396?openFiles=controllers.application.js%2C

I realize this is an edge case. I couldn't get a twiddle that created the full problem which is this: In a form that has a submit with a text box element that has an oninput action which is put on the back-burner via Ember.run.later() -- When the submit button is clicked somehow the oninput would fire (or possibly the back-burner would think that the action for oninput should fire at this time).

This was a bugger to track down. We are using the back burner so our webservice doesn't get pelted as users type a search is performed on the web-service. Forcing a delay via the back-burner solved the problem when users would speed type. Part of the code in the oninput action is to null some properties. When a user would hit the submit button all the data was getting clobbered because the action on the back-burner would unexpectedly execute even though the user hadn't typed anything into that textbox.

Serabe commented 6 years ago

Hello, @RyanNerd,

this is not a bug. Though the input is removed from DOM, while it was still in, you asked Ember to run later a piece of code. This piece of code is being cancel each time you call codeDidChange but not while toggling it. See the fix in this twiddle

For complex, async tasks let me recommend you the amazing ember-concurrency addon.

Thank you!

RyanNerd commented 6 years ago

@Serabe Please re-open. The issue is that the oninput action is firing in the back-burner (EVEN WHEN NO INPUT EVENTS ARE QUEUED).

This happens on a form when submit action takes place and the form is getting torn down.

My suspicion is that when the form is closing the back-burner code is executing all functions in the back-burner to clear the queue (it doesn't matter if the user has typed something or not the onchange action is on the backburner and the action code is firing even though no oninput activity/event has occurred). Unfortunately this has a side effect in the case I stated above -- that even though there are no pending events for the oninput action the action code for oninput fires anyway when the element goes out of scope. This shouldn't happen and is a bug.

Serabe commented 6 years ago

The bug in the original twiddle is a bug in the implementation. It looks like the code processes the events/handlers like so...

When the input event is fire, the method in actions named codeChanged is called with its context set to the component. That method calls the method codeDidChanged in the component. codeDidChanged first cancels previous task and then sets a new one. That task is not run after 800ms. codeDidChanged returns. codeChanged returns. After 800ms, a task is run. Nor Ember nor JavaScript knows nor should know that that task should not be run if the input is no more in the DOM. Think that it can be an analytics script debounced or any other thing.

I modified the original twiddle to show the log of what's happening.

I've tried just toggling the input without typing and typing, waiting for the task to be done, and then toggling, but nothing happened.

Perhaps with these twiddle examples you can reach out in the ember community slack #-help channel do discuss your use case. The community chat is very active.

If we can confirm there is a bug we can reopen the issue.

Thank you!