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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed an error that is caused by the timeout firing after the element has been removed from the DOM #591

Merged
merged 3 commits into from
Dec 10, 2013

Conversation

simpleTechs
Copy link

In some cases where a can-value bound select-element is inserted and then removed from the DOM in the same can.batch, the .set function may throw an error because this.element is not defined anymore, when the timeout from init finally fires.

@daffl
Copy link
Contributor

daffl commented Dec 9, 2013

Is there a way to test this? I can see that something goes wrong and wouldn't have too much of a problem merging without one but it would be nice to know when this happens.

@simpleTechs
Copy link
Author

Phew, let me look into my code and see if I can make it test out of it.

@simpleTechs
Copy link
Author

Hm, I'm not quite sure if the test is a good idea. It works, if you run it in the browser and with jQuery. It won't work with any other lib (due to the fact I needed jQuery's .remove in order to actually remove the DOM-node.)

If you have any idea how to do that without jQuery, please tell me. If I do it simply by using parent.removeChild() the node will be gone from DOM, but the binding will still have this.element; while using $('select').remove() does set this to null - thus, causing the issue.

@daffl
Copy link
Contributor

daffl commented Dec 10, 2013

Ah nice. I think can.remove should work for all libraries.

…e the timeout is firing after the element has been removed from the DOM
…ut is firing after the element has been removed from the DOM.
@simpleTechs
Copy link
Author

Awesome, thanks for the hint :-) I was able to build a working async-test now and even find another issue which I fixed with the latest commit. All should be fine now.

@daffl
Copy link
Contributor

daffl commented Dec 10, 2013

Awesome, thank you for catching that!

daffl added a commit that referenced this pull request Dec 10, 2013
fixed an error that is caused by the timeout firing after the element has been removed from the DOM
@daffl daffl merged commit aae41de into canjs:master Dec 10, 2013
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

Successfully merging this pull request may close these issues.

3 participants