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

Avoid infinite render on value change #524

Merged
merged 3 commits into from
Aug 31, 2017

Conversation

steven-ferguson
Copy link
Contributor

@steven-ferguson steven-ferguson commented Aug 25, 2017

Fixes #480 . Looks like the issue was closed even though a fix was never merged.

@jamesdaniels
Copy link
Contributor

We had to roll this back, due to #484. Any thoughts on how to fix the render + not break acceptance tests?

@steven-ferguson
Copy link
Contributor Author

steven-ferguson commented Aug 26, 2017

I had not seen that issue thanks for linking. One thing I am having trouble understanding is why we are not just returning ref.once('value') as a promise from _fetch:

_fetch(ref) {
  return ref.once('value');
}

@steven-ferguson
Copy link
Contributor Author

I have added a test which connects to a live firebase since that seems to be where the run loop issues were coming from. This test failed as expected with my initial commit, and now passes when only returning the promise from ref.once('value').

});

}, log);
return ref.once('value');
Copy link
Contributor

Choose a reason for hiding this comment

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

The log input param is not used. Try this:

return Promise.resolve(ref.once('value'), log);

@steven-ferguson
Copy link
Contributor Author

Ok updated. Let me know if you want to squash this down to one commit.

@jamesdaniels
Copy link
Contributor

We can squash from the pull request. Tests are red though @steven-ferguson can you take a peek?

@steven-ferguson
Copy link
Contributor Author

I was not able to reproduce the failures on my local machine. Would you mind kicking the build off again?

@tstirrat
Copy link
Contributor

I kicked it off again, let's see how that goes.

@tstirrat tstirrat merged commit 7fbe850 into FirebaseExtended:master Aug 31, 2017
@tstirrat
Copy link
Contributor

Pushing out a release, btw.

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.

Ember 2.11 : getting infinite rendering when loading a model with relationships
3 participants