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

Promise aware to one object proxy #142

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

aars
Copy link
Collaborator

@aars aars commented Mar 8, 2017

This is somewhat related to #141.

related proxy's are promise/able, except when they're not. Empty relationships do not support promises, so you can't always go model.get('some-relationship').then(relationship => { if (relationship) { // we have a relationship }) because there is no then for empty relationships.

Again, no tests for this (except that the current tests don't break). I will eventually add some, but I need to get my actual Ember app going, so I'm currently just collecting bugs and fixes. (if all these PRs and communication is not useful I can stack all these changes/bugs/issues/fixes on my own fork and discuss them later).

@pixelhandler
Copy link
Owner

@andrewmp1 originally the different between an relationship with a promise and one without was our apps flow for creating new records. In that case we were adding relationships that were not promise aware just referenced in the relationships property (object) of the model.

@aars I think you are correct there is a gap between an empty relation and an async one. Maybe all relations should use the proxy to be consistent. An empty relationship could be the promise proxy but resolved with content as null

@aars
Copy link
Collaborator Author

aars commented Mar 20, 2017

@pixelhandler Going with a proxy for empty objects will also tackle the "cached relationship" issue. Good call. I'll send in a new PR for that.

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.

2 participants