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

[BUGFIX BETA] Added system/store/container-instance-cache to the -private export #5002

Merged
merged 1 commit into from
Jun 1, 2017

Conversation

workmanw
Copy link

I'm sure adding more things to the -private export is 😢 . Unfortunately we need access to this class in ember-data-model-fragments. There was previously a discussion about alternative approaches, but we settled on monkey patching this class (adopted-ember-addons/ember-data-model-fragments#224).

@stefanpenner
Copy link
Member

@workmanw can you add a test, so we can be sure to not regress?

@workmanw workmanw force-pushed the instancecache-private-export branch from 27af46b to 8d3f4aa Compare May 30, 2017 18:44
@workmanw
Copy link
Author

@stefanpenner Added. There wasn't really presidence for this, and short of writing some tests that touch the interface of this, I didn't know what more to test than assert.ok(!!ContainerInstanceCache). Certainly open to feedback here.

I also added two more tests for InstanceModel and RootStates, both consumed by ember-data-model-fragments.

@stefanpenner
Copy link
Member

@workmanw awesome thanks. One last request, if you can leave a comment as to who is depending on this private API. It wont be a mystery for future travelers.

@workmanw
Copy link
Author

workmanw commented May 30, 2017

@stefanpenner Good idea. Would you prefer to see it in the -private/index.js file or the tests?

@stefanpenner
Copy link
Member

@stefanpenner Good idea. Would you prefer to see it in the -private/index.js file or the tests?

file

@workmanw workmanw force-pushed the instancecache-private-export branch from 8d3f4aa to 96a9208 Compare May 30, 2017 20:30
@workmanw
Copy link
Author

👍 Done. Thank you.

@bmac bmac merged commit 27d2aec into emberjs:master Jun 1, 2017
@bmac
Copy link
Member

bmac commented Jun 1, 2017

Thanks @workmanw.

@workmanw
Copy link
Author

workmanw commented Jun 5, 2017

Thanks @bmac! Any chance I could talk you into cherry-picking this into the beta branch sometime in the next few days? That way I can get the model-fragments beta tests working for the next release.

@bmac
Copy link
Member

bmac commented Jun 7, 2017

@workmanw I just published v2.14.0-beta.3.

@workmanw
Copy link
Author

workmanw commented Jun 7, 2017

@bmac Thank you!!!

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.

3 participants