Closed josephsavona closed 7 years ago
How pluggable would this be? I can imagine situations where I as a user do not need this extended flexibility. I'm worried that adding this as a core part of relay bloats the project and makes adoption and maintenance harder (for both users and developers).
I'd propose to bundle this work into a new project: A client-side GraphQL proxy that takes care of analyzing GraphQL queries and can dispatches parts/fragments to individual endpoints. The custom network layer should be powerful enough to allow this kind of integration!
@clentfort Those are natural concerns, thanks for bringing them up! Implementing local state management at the network layer has a number of problems, including the fact that all client data would be duplicated (once at the network layer, once in the Relay cache), it would force local data changes to use the heavyweight Relay.Mutation
s for what should be lightweight changes (for example, optimistic payloads aren't necessary when you're directly changing the source of truth), and it would prevent a variety of runtime optimizations that are critical in practice. In short: this needs to be in the framework.
I'm worried that adding this as a core part of relay bloats the project
We're confident that this enhancement will have the exact opposite effect: a solid architecture for non-server data sources will actually lead us to modularize the codebase even further.
Implementing local state management at the network layer has a number of problems, including the fact that all client data would be duplicated (once at the network layer, once in the Relay cache)
This might be true for pure in-memory data but not for any kind of device data which will be stored somewhere else. Having relay manage this kind of data sources adds tons of unneeded complexity! To eliminate problems with cache duplication it might be feasible to add additional meta-data to responses from the proxy that instruments Relay to not cache data. (Which might be a general cool feature to have for data that should always be fetched from the server.)
It would force local data changes to use the heavyweight
Relay.Mutation
s for what should be lightweight changes (for example, optimistic payloads aren't necessary when you're directly changing the source of truth)
This would require to have at least a secondary API for writing mutations of in-memory data, nullifying some of the benefits of using GraphQL in the first place. To solve the problem of optimistic payloads Relay could be changed to accept a stream of responses for a request: If a optimistic update is needed the proxy could just generate a response and "send" it to Relay immediately, than gather responses from all its data-sources and pass those on. If no optimistic update is needed because it was only an in-memory change the proxy can generate that answer immediately. (I know this would move optimistic updates out of Relay for situations where the proxy is used, but at the same time the proxy would have more knowledge about how an optimistic update should look like).
It would prevent a variety of runtime optimizations that are critical in practice
Could you elaborate which optimizations?
@clentfort thanks for the feedback and ideas :-) We'll be sure to engage with the community about architectural choices via follow-up issues and pull-requests. However, for now I'd like to refocus the discussion on concrete use cases in order to fully define the problem first.
A use case that we've considered is validation of client-only data. For example, when a user creates a draft of a post to a group, it's invalid if they lack privileges to post to that group. In these cases, the validity of the draft is a function of the draft itself (local data) and user/system settings (server data). Further, any change to either input source should ultimately update any views displaying the draft's validity. We've been referring to this as composed state.
Are there other examples of composed state? Is there a need for arbitrary levels of composition, or is data source -> compose -> react view
sufficient?
cc @vladar who had asked about client-side data in #106
@josephsavona My current case (real world example):
Consider thread of comments. Single comment can be "focused" or not. Focused comment will be highlighted + there is separate non-child component in the sidebar that displays full details about comment author.
So what I need effectively is:
So this is pretty much transient local state. It must be shared because required by different hierarchies or React components (thread and authorInfo).
Ideally, with Relay-only solution I would write query like this:
query {
activeThread {
focusedComment {
author {
...Fields
}
}
}
}
I guess, I would also want to add some directive
to query to mark field as local state.
But even if you can implement something like this for querying - there is another question about handing local state transitions. They may require some state transition engine behind them (like Flux store / state reducer).
Also they may be "mixed" - say local state changed when some mutation is executed that also mutates data on the server.
There are indeed many edge cases here.
A common use case for this would be dealing with non-GraphQL data sources. We use an on-device service to store data about current device state. The data is pushed to the client via a socket connection. We wind up storing this in Flux stores and then merging Relay data and Flux data in the container, which is fairly error-prone and complex.
Currently we have something like this:
query {
app {
id,
display_name,
description
...
and:
AppFluxStore<List<App>>
App: {id, isInstalled, isDownloading, etc}
In order to render the current app state, we need to resolve the GraphQL query, find the appropriate matching id in the FluxStore, and merge the information together.
One solution would be to have local fields that: 1) have a getter method that can retrieve arbitrary local data based on query results (id in the case above). essentially they can "augment" server-side data with local state once a query is resolved 2) can subscribe to change events in a FluxStore or to arbitrary event emitters.
This would open up a path for relatively easy migration from Flux-based apps to Relay.
@ykagan that's a great example of the uses-cases we've looked at. It's common for products to have views that compose server & local data. Validation is another example: item data is local, but rules about validity come from the server.
By looking at the relay based mvc example that runs grpahql on client-side... I think it is very interesting to run a grpahql service on client-side that would bridge to non-graphql sources. That way, the application would talk to all sorts of data via graphql.
I was going to try an implementation where I inject a customized network layer that will call the default fetcher for specific queries like 'query { viewer}'. And use the client-side graphql service for queries like 'query { cache }'. For transpiling purposes, i'd merge the 2 schemas and use that combined schema for transpiling. However, both schemas contain their own 'node' field, which (haven't tested this yet) might run into issue.
I think the 'multiple schema' usage mentioned in #130 might actually offer a way to do local caching with graphql?
Executing GraphQL queries that go to remotes sounds like a bad idea to me. For example, it's highlighted explicitly as not being recommended for production on the relay-local-schema package.
A GraphQL gateway running in a data center has the advantage of reliable, fast access to the data sources it's using, especially if they're also yours. The GraphQL query getting evaluated on your phone on browser has none of these advantages.
@taion Well yeah, but I could be using graphql and resolving to localStorage or indexDB completed on the device. Setting up the cache service like a backend on a webworker maybe, and let all the components fetch data via Relay regardless of fetching from backend or fetching from local cache.
But I guess that's added work to write the cache service with graphql if we can use localStorage from the beginning. Not to mention more overhead for simply storing some data on client-side.
Just a thought anyway...
Oh, I misunderstood what you said. Never mind. That makes sense.
I'm working on an app that uses DOM elements as a data source. The DOM elements are on the same web page as my React UX, but they are not rendered by React.
My app is a JS script that can be added to a website to enhance it's functionality. Some Chrome extensions might want to do something similar.
I realize my use-case is pretty niche, but I would love to be able to write out my React component data needs in GraphQL and have a relatively easy way to fill in those needs with arbitrary data that is already available in memory (DOM elements in my case).
Currently I use propTypes instead. I have custom code that reads the component propTypes and uses them to provide the components with their data.
@ahfarmer It's possible to polyfill this today by injecting a custom network layer, and then running a GraphQL schema in the browser. This is actually how the Relay Playground works - take a look at the source, in particular the use of relay-local-schema
.
Is someone working on this right now? We need it badly.
Our use-case is VERY similar to @vladar's with focused comments.
@staugaard This is something we're continuing to explore; we aren't ready to release anything just yet. However, in the meantime there is a very flexible alternative: use a custom network layer. You can inject a custom network layer that runs a GraphQL schema in the browser and resolves queries using a combination of local and remote data. This is actually how the Relay Playground works, using relay-local-schema
.
I am working on a RelayCompositeNetworkLayer
at the moment. The idea being that you might have multiple schemas and you want to send each to a different network layer. The 99% use case is 'server' data and 'local' data.
Relay.injectNetworkLayer(new RelayCompositeNetworkLayer({
...config,
layers: {
server: new Relay.DefaultNetworkLayer('/graphql'),
local: new RelayLocalSchema.NetworkLayer({schema: localSchema})
}
}));
This works, but has a few downsides being at the network layer. So here is feedback on how it could be better if done as part of Relay.QL
babel plugin. Also, these downsides could just be specific to my implementation of the idea :).
In order to split the query the network layer (and therefore the client code) needs to know which schema each field belongs to. For example it needs to know User#drafts
is local
and User#name
is server
. This is fine for small schemas, but you can imagine for a large schema (e.g. facebook) this would be unworkable. Instead it would be nice to only download enough schema information as required. Relay.QL
could do this by adding metadata
to the query on which schema a field belongs to. You'd only get as much schema information as the client is currently rendering.
Given the following query where drafts
is local
but user
is server
:
query {
viewer {
name
draft {
text
author {
name
}
}
}
}
We would make three queries:
query { viewer { id, name } }
query { node(id: $viewerid) { ... on User { draft { text author { id } } } }
query { node(id: $authorid) { ... on User { name } } }
If the viewer
and author
are the same user, the third query is unnecessary -- the Relay.Store
has this information. At the network layer the third request is unavoidable. Instead if Relay.QL
assigned schema then the query could be split / diffed as part of the Relay.Store
workflow (e.g. similar to how the GraphQLQueryRunner
... I think ... does defer
stuff).
In order to get around this at the network layer you could modify the original RelayQuery so that the PendingQueryTracker
will send any overlapping dependent queries and then the network layer can call back into RelayStore.primeCache
resulting in only the smallest diffed queries being sent. Clearly this is a bad idea and would be better if this was done as part of the query running.
The network layer can only send back a single response. So if a query is split into three sub queries at the network layer no data can be returned until all queries have completed. If the queries were created by relay internals it could create multiple requests and resolve each individually (again, like defer
).
This isn't really specific to doing it at the network layer, but more of an issue with having multiple schemas / remotes. Again, another query:
query {
viewer {
name
drafts(first: 10) {
text
author {
name
}
}
}
}
This is a natural query to write but if drafts
are local and user
is remote then you could end up making 10 requests for author
-- one per draft.
So one idea is making the extension one-way. Local can extend server, but it can't have fields go back to server.
I think that sums up thoughts I've had on this so far. That said, doing it at the network layer works fine (so far) and it is pretty cool to query local / server data from the same query interface. Of course I have no idea how many edge cases my implementation is missing ;p
@eyston Great writeup! Here are some brief thoughts on the main points:
babel-relay-plugin
provides an opportunity to make this information available within the query representation, avoiding the need to download the schema at runtime.Another challenge is determining a simple API for mutating local state. By way of comparison, the Relay.Mutation
API describes complex client/server interactions and is necessarily somewhat complex.
Here is the code for the composite network layer I'm working on: https://github.com/eyston/relay-composite-network-layer
It works but has a huge limitation in that it only handles query cases I've thought of :)
Minor side note -- testing is hard because I don't know how to clear the Relay.Store
between tests so order matters and some queries could be using cached data ;p
Well that's cool.
On Mon, Dec 28, 2015 at 12:53 PM Huey Petersen notifications@github.com wrote:
Here is the code for the composite network layer I'm working on: https://github.com/eyston/relay-composite-network-layer
It works but has a huge limitation in that it only handles query cases I've thought of :)
Minor side note -- testing is hard because I don't know how to clear the Relay.Store between tests so order matters and some queries could be using cached data ;p
— Reply to this email directly or view it on GitHub https://github.com/facebook/relay/issues/114#issuecomment-167640891.
I think the simplest solution would be to use something like a Flux pattern for handling a client-side specific data. My initial idea was also about making a composition of the network layers. Very curious about how this issue will evolve.
Something else this would enable is being able to query public GraphQL servers from within your app e.g. those @ https://www.graphqlhub.com/
You could have, for example, a sidebar with the latest 10 HN posts.
being able to query public GraphQL servers from within your app
@KyleAMathews Do you mean query multiple public GraphQL servers? You could query a public server today by configuring a network layer.
Wait how would that work? On Tue, Feb 2, 2016 at 6:09 PM Joseph Savona notifications@github.com wrote:
being able to query public GraphQL servers from within your app
@KyleAMathews https://github.com/KyleAMathews Do you mean query multiple public GraphQL servers? You could query a public server today by configuring a network layer.
— Reply to this email directly or view it on GitHub https://github.com/facebook/relay/issues/114#issuecomment-178957965.
@KyleAMathews configure a network layer and request data? what am i missing?
In my HN news example, Relay would need to know about two schemas, the one I control + the HN one and intelligently split out queries depending on what schema the query is for. So just like we'll be able to split out client-only queries, we'd also (I assume) be able to send queries to 3rd-party public GraphQL servers. AFAIK this isn't possible right now.
@en_JS
seems like you already sorted out what needs to happen...
from the "Future" section of the Relay Roadmap
Support client/local state API for resolving fields locally: #431. Support querying & compiling client-only fields by extending the server schema, and a means for writing data for these fields into the cache: #114.
:+1:
First off, thanks for open sourcing Relay. There's some really clever ideas in this project, and we really appreciate all the work the team has done.
We're currently evaluating Relay, and how to elegantly combine client state and events with Relay server data is the biggest unanswered question (we've looked at Stackoverflow and the issues here).
@josephsavona What's the recommended way of doing this in production today? We think that nailing this will be one of the best indicators of how well Relay fits our needs. It appears to be the biggest weakness (and potential risk) with using Relay before this issue is resolved.
I'm assuming Facebook tackled this problem in some way for the complex UI state in AdsManager?
Side note: We'd love to see a blog post on best practices regarding Schema design :)
We're in the same boat. I hear people are using Redux Forms and ad-hock solutions for local state and while that sounds fine I'm curious if the story gets better when client-only state can be supported as first class citizen. I think it may be far more important conceptually than technically necessary.
client state and events with Relay ... What's the recommended way of doing this in production today?
@jimkyndemeyer Great question. For many of our Relay apps we haven't needed any separate client-side state solution - React component state plus Relay has been sufficient. If your app is complex enough to need a separate solution for client state, we'd recommend pairing Relay with Flux (Redux). You can fetch & render server data with Relay and manage your local state with Redux. Check out recompose
which has some great helpers for working with both Redux and Relay. Finally, if you need access to server data outside of a container (e.g. in an action creator), you can use Relay's imperative data-fetching API to request it:
var query = Relay.createQuery(Relay.QL`query { ... }`, {var: 'value'});
Relay.Store.primeCache({query}, ({done, error}) => {
if (done) {
const data = Relay.Store.readQuery(query)[0];
// ... do stuff with `data`
} else if (error) {
// handle error...
}
});
It's pretty easy to wrap that snippet in a Promise-returning function e.g. for use with redux-saga
.
IMO, if you mix in too much of the imperative approach to state management you go back to square one. When you need to change your UI you'll have a lot of rework.
@josephsavona I had captured the following after talking to you a while back. Is this still in the roadmap?
Any app state that is not sync'd to the db is not something that Relay encompasses right now, but there is an ongoing discussion for handling scenarios like client-side form validation and state updates from sources other than the db (e.g. websocket)
These important scenarios will be addressed according to the Relay Roadmap (https://github.com/facebook/relay/wiki/Roadmap):
API for resolving fields locally: #431.
Support querying & compiling client-only fields by extending the server schema, and a means for writing data for these fields into the cache: #114.
@idibidiart at some level data-fetching will be imperative. Action creators are the natural place to do this in redux, and data-fetching can even be accomplished in a functional/declarative style via redux-saga.
I had captured the following after talking to you a while back...is this still in the roadmap?
Unfortunately I haven't had time to review your gist that you're citing, so I can't confirm to what extent it reflects our thoughts/plans. This specific issue (local state) is definitely something we would like to solve at some point, hence the ongoing discussion here.
@josephsavona Thanks for the info. I looked it over, and we'll definitely use the imperative API for ad-hoc searches etc.
We're looking for an elegant way of synchronizing local shared state with the Relay variables that are kept inside Relay containers. The use case is similar to what @vladar and @staugaard posted.
Let me give you an example based on the TodoMVC app by adding a new feature:
Imagine the user can click on a todo to select it. On the right hand side, there should be a Relay container that displays additional details about the selected todo.
To make sure this happens we need shared state, e.g. a selectedTodoId
property in a store. The Todo
component can dispatch a SELECT_TODO
action, and our Relay(TodoDetails)
container needs to use the selectedTodoId
value as a variable to query the details of the todo.
From what I can find in the Relay Container API, the way to do this is to use this.props.relay.setVariables
.
The trick is to call setVariables
each time any relevant local state changes. I found an example of calling setVariables to keep them in sync at fdecampredon/f337605e393a5b032b85#file-redux-relay-js-L73-L80
We've arrived at something similar in our prototyping: An additional HoC that is placed in-between a Relay container and the component that needs the Relay data as props. So something like:
Relay( ClientStateRelayVariablesListener( ComponentThatNeedsData ) )
ClientStateRelayVariablesListener
does the following:
ComponentThatNeedsData
by forwarding all propsthis.props.relay.variables
. If there is a match it calls this.props.relay.setVariables
on behalf of ComponentThatNeedsData
That's the basic gist of what we're doing. The actual implementation is a bit more complicated since setVariables
is async, so we queue up the local state changes and only signal them once the Relay data comes back
While the query is running we set a "(variableName)IsLoading" boolean on the store, e.g. selectedTodoIdIsLoading
. This allows the details component to render a spinner. As a side note we use mobx to signal the names and values of properties that change on the store.
I guess I'm looking for some kind of feedback as to whether this approach is the way to go?
We don't want to overlook something in the current Relay API, or over-engineer this. Our goal is to provide a great DX where our team doesn't have to also write setVariables boilerplate code each time they mutate shared local state.
I think you already are over-engineering that – such cases are probably handled by a component wrapping a new Relay RootContainer fetching just the required data. Logically, it's not dissimilar to showing a modal or a popover that has its own data requirements.
I agree with @talon here
@josephsavona : those were capturing what the Relay Roadmap stated, not my gist. Sorry for the confusion. Your Relay roadmap had stated at some point these ideas/features were being pursued:
API for resolving fields locally: #431.
Support querying & compiling client-only fields by extending the server schema, and a means for writing data for these fields into the cache: #114.
Are these still on the Roadmap?
When it comes to the subject of client-only state and Relay, I think the two features listed in the Roadmap (see Update #1 below) are must-have eventually. But I see a risk at a conceptual level in what many people take for granted: that client's job is to specify to the server how to satisfy user intent -- This leads to all sorts of complications. App state that sync'd via Relay between the UI component tree and our graph model on the server should be all that is needed in most cases (other than client-only state say animation state and other client state that we don't care to sync to server, e.g having 5 windows open in Gmail) Client should relay mutations to that state that are due to user interaction and the server should figure out user intent and how to satisfy it. Having the client figure out user intent and how to satisfy it (and, specifically, having to keep stateful client-side logic to do that) means that you're isolating that part of the application's behavior in the client, which leads to a much heavier and more complicated client codebase.
@taion I don't really understand your answer. Can you outline how you would solve the scenario I outlined. I'd be more than happy to learn that we are in fact over-engineering this :)
Use a new <RootContainer>
. Generate a Relay route (possibly in componentWillReceiveProps
) when the injected selected ID gets updated from the store.
@taion Isn't that just an implementation detail? Whether we're using setVariables or updating a route, the challenge is to ensure that the store and the relevant Relay (root)containers alway stay in sync.
What we've done with the additional HoC is let developers say abstractly, "My Relay variable "foo" should always stay in sync with property "foo" on store "Bar". The HoC then takes care of the subscription logic, e..g remembering to unsubscribe in unmount. Without the HoC that code is scattered throughout the component tree.
Sort of – you get all the "subscription" handling for free with either React context or Redux containers, though, so it's not trivial.
I have a use case that can be easily explained as a TodoList app. For apps like TodoList, it needs to be able to work when user is offline. Users should be able to see (or query) a list of todos, add (or mutate) a new todo, mark a todo as complete, or remove a todo, ... regardless of their online status. Then, when user comes back online it needs to sync the local data with the server data. So, majority of the schema is shared between server and local, while there could be some operations or parts of schema that require online status such as managing notification settings, changing user password, etc. Thus, there could be eventually two graphql servers, remote and local (graphql on top of Realm DB running locally).
While hacking this week we realized that we had most of the pieces necessary to support client state (as part of a larger update to Relay core). We'll go into more detail on the wider changes in a blog post next week-ish and some upcoming conference talks, but here's what we are thinking for local state.
Developers can specify extensions to the server schema within their GraphQL queries, using the schema definition language. For example:
const fragment = RelayQL`
fragment on Story {
text # server field
hasViewerRead # client field
}
extend type Story {
hasViewerRead: Boolean
}
`;
The hasViewerRead
field will be removed at query "compile" time, so that this field isn't queried on the server (which doesn't know about the field). On the client, developers can write arbitrary logic to mutate the contents of the store - like Flux/Redux actions. We're still iterating on this API, but it would be along the lines of:
RelayStore.run(store => {
store.get(storyId).setValue('hasViewerRead', true);
});
UI components would automatically be subscribed and have access to these fields via RelayContainer
fragments. So whenever the value of hasViewerRead
changes, components would re-render.
There are lots of things for us to work out. Can we support client only state (w/o any server schema at all)? How about intercepting server payloads to set default values for client fields? We're not sure! We look forward to exploring these with the community over the coming months.
For now, keep your eyes out for that blog post ;-)
@josephsavona do you have any updates? Getting proper support for local state in Relay is huge (no pun intended)!
There is no branch to track how things progress, which I assume is because of Facebooks from Internal use to OSS release workflow.
@hkjorgensen No updates yet. We're actively working on building out the new core, and will make it open-source once we're confident about using it in production. In the meantime, we hope to share more about Relay local state in the upcoming talks mentioned in our recent blog post.
Removing the "inprogress" label because nobody is explicitly working on this right now. Rather, we're focusing on rolling out the new core and APIs, and these do have support for declaring and using client fields, albeit at a low-level; we still have to figure out the right way to expose this functionality in the higher-level API.
In fact, I think it makes the most sense to close this one, where some of the commentary is now outdated, and instead make a new issue that explicit targets making the existing client-field capability in the new core exposed in an ergonomic way.
As mentioned in the introductory blog post, we're exploring ways to extend Relay to represent data from multiple sources. These data sources could be in-memory objects, native device APIs, or a (GraphQL) server.
The goal is to allow a unified programming model for accessing all the information relevant to an application - while also retaining a few important invariants:
Proposed API
The proposed API for product developers is the same GraphQL fragments that we use today. Products can define a unified schema of all of their data, and query it as you would expect:
Some considerations here are how to namespace fields & types to avoid collisions between different data sources, the API for registering non-server types, and the API for updating non-server data.
RFC
We'd appreciate your input about possible use-cases for this feature. Some cases that we have considered are retrieving a list of photos via native devices APIs or storing a list of in-progress drafts/edits.