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

selectionChanged loop in 2.2.2 #151

Open
orf opened this issue Sep 14, 2016 · 6 comments
Open

selectionChanged loop in 2.2.2 #151

orf opened this issue Sep 14, 2016 · 6 comments
Milestone

Comments

@orf
Copy link

orf commented Sep 14, 2016

Version of x-select: 2.2.2

Ember Version / Ember CLI Version:

Ember Version: 2.6
Ember CLI Version: 2.6

Expected Behavior

Initializing an x-select should not immediately trigger the changed action

Actual Behavior

Initializing an x-select immediately triggers the changed action

Info

We initialize an x-select component with the value set to a models attribute, foo.bar. Our action bubbles up and eventually triggers a save on the model, which worked absolutely fine until version 2.2.2 of this addon.

In 2.2.2 the change event is immediately triggered, which causes an infinite loop where the event causes a save, which then causes the property to change and the change event to fire again for some reason. We hide the select box while the model is saving (I think this is a fairly common pattern), which destroys the component. When the model finishes saving the component is re-created, which then again triggers the save event.

It is caused by this commit: ea95225

@Robdel12
Copy link
Collaborator

Hi @orf! I think the first step would be to stop using action and use onblur instead. This way the action will only fire when the select loses focus.

I think #149 helps tremendously in this aspect too, but that's probably only going to make it into 3.0.

@cowboyd
Copy link
Collaborator

cowboyd commented Sep 14, 2016

This usually happens because a change is, in fact, occuring.

Remember that the value of a select must always be one of its options, so if the value you pass in is null and you don't provide a null option, then it will select the first option provided resulting in a change and a resulting action fire.

You can always have a null option. If this does not work, then it is definitely a bug. I.e. a value of null and first option of null, should not result in an action being fired.

@orf
Copy link
Author

orf commented Sep 15, 2016

Thanks for your replies!

@cowboyd: In this case a change isn't occurring, and there is a null option in the first position ({{#x-option value=null}}----{{/x-option}}). I added a {{log}} helper to the template and it fires exactly once, the first initial time.

@Robdel12: using onblur stops this from happening, thanks for that, but in our specific case the action is a much better UI choice. It's very plausible that someone would change the value of the select box and close the window or walk away without the event triggering.

It seems to happen like this:

Each option calls registerOption, which then calls _setDefaultValues once per option. This then repeatedly triggers the action. So if you have 4 options then action will be triggered 4 times with the result of _getValue(). Each of those action calls triggers a save, which reloads the component and then triggers another 4 actions. This results in a quite impressive DDOS of both the server and the browser!

I really really really don't have time to investigate further (we are shipping on Monday!) but I think it's definitely a bug or regression caused by ea95225. I can maybe create a fiddle next week if it's not too crazy, but until then I'm just going to pin @ 2.2.1.

@cowboyd
Copy link
Collaborator

cowboyd commented Sep 15, 2016

Null firing on null? That's definitely a bug!

On Sep 15, 2016, at 03:44, orf [email protected] wrote:

Thanks for your replies!

@cowboyd: In this case a change isn't occurring, and there is a null option in the first position ({{#x-option value=null}}----{{/x-option}}). I added a {{log}} helper to the template and it fires exactly once, the first initial time.

@Robdel12: using onblur stops this from happening, thanks for that, but in our specific case the action is a much better UI choice. It's very plausible that someone would change the value of the select box and close the window or walk away without the event triggering.

It seems to happen like this:

Each option calls registerOption, which then calls _setDefaultValues once per option. This then repeatedly triggers the action. So if you have 4 options then action will be triggered 4 times with the result of _getValue().

I really really really don't have time to investigate further (we are shipping on Monday!) but I think it's a bug caused by ea95225. I can maybe create a fiddle next week if it's not too crazy, but until then I'm just going to pin @ 2.2.1.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@ghost
Copy link

ghost commented Oct 19, 2016

seeing this behavior from [email protected] in my project's integration tests

@kjhangiani
Copy link

A year later, but ran into this issue when attempting to upgrade from 2.0.2 to 2.2.3+. Currently, forced to fork and wrap an else around the sendAction call from the commit specified by @Robdel12

Hopefully once we are able to fully migrate and test our components with pure one-way, we can revert back to the main branch.

kjhangiani added a commit to soxhub/emberx-select-backup that referenced this issue Dec 1, 2017
@Robdel12 Robdel12 added this to the 3.1.2 milestone Feb 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants