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

[v2 backport] Pass owner instead of registry to ember-data's setupContainer #1388

Open
wants to merge 2 commits into
base: v2-x
Choose a base branch
from

Conversation

anehx
Copy link
Contributor

@anehx anehx commented May 12, 2023

The setupContainer function of ember-data expects an application instance to be passed - in this case that is the faked owner object that we create when using a custom resolver in tests.

This behaviour previously worked because ember-data supported the usage of legacy function optionsForType which exists on the registry object. However, they removed that legacy fallback in v4 and replaced it with registerOptionsForType which does not exist on the registry but only on the owner.

Resolves #1386

The `setupContainer` function of `ember-data` expects an application
instance to be passed - in this case that is the faked owner object that
we create when using a custom resolver in tests.

This behaviour previously worked because `ember-data` supported the
usage of legacy function `optionsForType` which exists on the `registry`
object. However, they removed that legacy fallback in v4 and replaced it
with `registerOptionsForType` which does not exist on the registry but
only on the owner.

Resolves emberjs#1386
@MelSumner MelSumner requested a review from chriskrycho May 30, 2023 23:35
@anehx anehx force-pushed the fix-setup-container-usage-backport branch from 9ea2f49 to 8867c01 Compare May 31, 2023 08:07
@anehx anehx force-pushed the fix-setup-container-usage-backport branch from 8867c01 to 9500d0b Compare May 31, 2023 09:11
@anehx
Copy link
Contributor Author

anehx commented May 31, 2023

@chriskrycho @MelSumner The tests for release, beta and canary will always fail here since this v2 of this addon still uses assign from @ember/polyfills which was removed in v5. However, I don't see the reason for the failing LTS tests.

Can someone support me with this? We currently can't update ember-data in various projects because of this issue, so this is important to us. Or is there already a release for v3 planned?

@@ -118,7 +118,7 @@ export default function (resolver: Resolver) {
// correctly for the tests; that's why we import and call setupContainer
// here; also see https://github.com/emberjs/data/issues/4071 for context
let setupContainer = require('ember-data/setup-container')['default'];
setupContainer(registry || container);
setupContainer(owner);
Copy link
Contributor

Choose a reason for hiding this comment

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

This likely needs some variety of version detection to pass the appropriate bits from Ember depending on its version (and potentially depending on the Ember Data version as well?). I don't have the relevant context there, but based on the test failures for older versions of Ember, I think that's the likely issue.

However, given v3 is out now, it is almost certainly better to upgrade to that instead of trying to do this, if at all possible!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants