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

Don't update value if tearing component down #97

Merged
merged 5 commits into from
Mar 9, 2016

Conversation

Robdel12
Copy link
Collaborator

Related to #94

We don't want to set the value of the component if we're destroying the component. This is what's happening in #94.

TODOs:

  • Write test to prevent this from happening again
  • Verify with @fivetanley / @cowboyd this is the right solution.

@Robdel12 Robdel12 added this to the v2.1.0 milestone Feb 28, 2016

// We don't want to update the value if we're tearing the component down.
if (!this.get('isDestroying')) {
this._updateValueSingle();
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also should this check if multiple=true is set and do this._updateValueMultiple()?

if (!this.get('isDestroying')) {
  if (this.get('multiple')) {
    this._updateValueMultiple();
  } else {
    this.updateValueSingle();
  }
} 

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Robdel12: Yeah you should check that! Just wanted to add a PR to fix the multiple=true case. Without the fix the value will change from an array to one single value, as soon as the component tears down.

Btw just use this._setDefaultValues() instead of this._updateValueSingle(), no need to write duplicate code.

@Robdel12 Robdel12 assigned cowboyd and unassigned fivetanley Mar 9, 2016
@Robdel12 Robdel12 changed the title WIP: Don't update value if tearing component down Don't update value if tearing component down Mar 9, 2016
cowboyd added a commit that referenced this pull request Mar 9, 2016
Don't update value if tearing component down
@cowboyd cowboyd merged commit c923be9 into master Mar 9, 2016
@cowboyd cowboyd deleted the ember-data-rollback-error branch March 9, 2016 18:35
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.

None yet

4 participants