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

Place store on Ember.Route to maintain implicit record loading #7765

Closed
wants to merge 7 commits into from

Conversation

snewcomer
Copy link
Contributor

@snewcomer snewcomer added 🏷️ chore This PR primarily refactors code or updates dependencies backport-beta PR targets the beta branch labels Nov 20, 2021
@snewcomer snewcomer self-assigned this Nov 20, 2021
@snewcomer snewcomer force-pushed the sn/inject-store-route branch from 4d4d9be to 3a69d25 Compare November 20, 2021 03:07
@snewcomer snewcomer added 🎯 beta PR should be backported to beta and removed 🎯 beta PR should be backported to beta labels Nov 21, 2021
@snewcomer snewcomer force-pushed the sn/inject-store-route branch from 3f08041 to ec86df7 Compare November 21, 2021 21:50
@@ -46,6 +51,22 @@ function initializeStore(application) {
}
}

// Implicit injection was removed. This is a replacement for Ember Route implicit store
// https://github.com/emberjs/rfcs/pull/774
Object.defineProperty(Route, 'store', {
Copy link
Member

Choose a reason for hiding this comment

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

This would define a property on the Route class itself, right? Why not use reopen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You were right. I has thought defineProperty and reopen would be equivalent but turns out I was wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm guessing this has something to do with EmberObject. But overall I was trying the other way to avoid the reopen deprecation

@igorT
Copy link
Member

igorT commented Nov 24, 2021

Once we fix the implicit injection deprecation for 3.28 do we still need to do this?

@snewcomer snewcomer force-pushed the sn/inject-store-route branch from 0770a28 to 25af494 Compare November 24, 2021 14:53
@snewcomer snewcomer requested a review from igorT November 24, 2021 18:08
@snewcomer snewcomer requested a review from mixonic November 24, 2021 18:13
@snewcomer snewcomer closed this Nov 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-beta PR targets the beta branch 🏷️ chore This PR primarily refactors code or updates dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants