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.

Sign up for GitHub

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

Jump to bottom

Add RelayContext #698

Closed
wants to merge 3 commits into from
Closed

Add RelayContext #698

wants to merge 3 commits into from

Conversation

denvned
Copy link
Contributor

@denvned denvned commented Dec 22, 2015

This PR is another step toward making all Relay state contextual (#558). Originally this was submitted as a part of #683, but @josephsavona suggested to split it to smaller PRs.

@josephsavona
Copy link
Contributor

Awesome, quick turnaround! I'll review in the morning but at a quick glance this looks great.

josephsavona
josephsavona reviewed Dec 22, 2015
View reviewed changes
src/store/RelayContext.js
RelayQuerySet,
} from 'RelayInternalTypes';

let defaultInstance;
Copy link
Contributor

Choose a reason for hiding this comment

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

let's use RelayStore as the default instance, so we don't need a default instance or getDefaultInstance here

@josephsavona
Copy link
Contributor

Sweet, this looks great. Main theme of my feedback is that we should use RelayStore as the "default" RelayContext instance to aid in the transition: all existing Relay codebases are already targeting that module so they'll continue to work.

@denvned
Copy link
Contributor Author

denvned commented Dec 22, 2015

@josephsavona Committed your suggestions. Did I understand everything right?

@josephsavona
Copy link
Contributor

nice job with this. thanks!

@josephsavona
Copy link
Contributor

@facebook-github-bot import

@facebook-github-bot
Copy link
Contributor

Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/1668105273478048/int_phab to review.

This was referenced Dec 31, 2015
Closed
Closed
@denvned
Merge remote-tracking branch 'upstream/master' into add-context
9ae01e9 
Conflicts:
	src/store/RelayStore.js
	src/store/__tests__/RelayContext-test.js
@ghost ghost closed this in 786ff5a Jan 20, 2016
This was referenced May 25, 2021
Open
Open
Open
Open
@joseroubert08 joseroubert08 mentioned this pull request Nov 28, 2023
Open
@enterstudio enterstudio mentioned this pull request Nov 30, 2023
Open
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
CLA Signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@denvned @josephsavona @facebook-github-bot