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

feat: allow .load()ing sync relationships #7668

Merged
merged 2 commits into from
Dec 7, 2022

Conversation

machty
Copy link
Contributor

@machty machty commented Aug 20, 2021

I think it should be possible to call .load() on a sync belongsTo reference,
but if you do, you get an AssertionError:

Assertion Failed: You looked up the 'team' relationship on a 'person' with id 1
but some of the associated records were not loaded. Either make sure they are
all loaded together with the parent record, or specify that the relationship
is async (belongsTo({ async: true }))

This commit adds a breaking test to demonstrate this.

Might be worth checking sync hasMany's handling of this too.

Note: for some reason, .reload() works (e.g. model.belongsTo('foo').reload()) but not .load().

@runspired
Copy link
Contributor

this is a feature request, albeit a relatively minor one which we can probably skip the RFC on. The EmberDataStoreFront pattern mostly used reload here to work around this. The reason this fails is because we take the same codepath for sync relationships on load as if you'd simply accessed the relationship off of the record. That codepath is guarded because (as the error message says) you're accessing it prior to having loaded it. In this case though, the intent is clearly to get around that and we should make it nice.

@runspired runspired added the 🏷️ feat This PR introduces a new feature label Apr 15, 2022
I think it should be possible to call .load() on a sync belongsTo reference,
but if you do, you get an AssertionError:

Assertion Failed: You looked up the 'team' relationship on a 'person' with id 1
but some of the associated records were not loaded. Either make sure they are
all loaded together with the parent record, or specify that the relationship
is async (`belongsTo({ async: true })`)

This commit adds a breaking test to demonstrate this.
@runspired runspired added the 🎯 canary PR is targeting canary (default) label Dec 7, 2022
@runspired runspired changed the title Breaking test for .load()ing sync belongsTo feat: allow .load()ing sync belongsTo Dec 7, 2022
@runspired runspired changed the title feat: allow .load()ing sync belongsTo feat: allow .load()ing sync relationships Dec 7, 2022
@runspired runspired merged commit fa8187a into emberjs:master Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎯 canary PR is targeting canary (default) 🏷️ feat This PR introduces a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants