-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Expose injectCacheManager on RelayEnvironment #1320
Conversation
it('passes down the cacheManager to the store data', () => { | ||
const mockCacheManager = {}; | ||
environment.injectCacheManager(mockCacheManager); | ||
expect(environment._storeData._cacheManager).toEqual(mockCacheManager); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
expect(environment.getStoreData().getCacheManager()).toBe(mockCacheManager);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, forgot about getStoreData()
and getCacheManager()
. Fixed 👍
06c1eb5
to
4ce48ea
Compare
@facebook-github-bot shipit |
Thanks for importing.If you are an FB employee go to Phabricator to review internal test results. |
@josephsavona do you know why the test is failing? is |
Not sure, I would try logging some of the values and see. |
const mockCacheManager = {}; | ||
environment.injectCacheManager(mockCacheManager); | ||
expect( | ||
environment.getStoreData().getCacheManager() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getCacheManager
don't seem to exist (there is no such method in the RelayStoreData class)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weird, thought we had that. Anyway, let's add and use it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can add it in this PR if you'd like
@facebook-github-bot shipit |
Thanks for importing.If you are an FB employee go to Phabricator to review internal test results. |
e9e3b24
@aweary thanks for this! |
RelayEnvironment
exposes most of the other injection methods forRelayStoreData
, so this just addsinjectCacheManager
as well. Even though it's not a documented API, adding it toRelayEnvironment
will make it easier for those who do want to use it while its "experimental" which paves the way for a stable implementation.I also added a basic test that just verifies the injection works as expected.