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

Update react relay modern ssr example #11193

Closed
wants to merge 2 commits into from
Closed

Update react relay modern ssr example #11193

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b4a5f0e
use a common store instance for init and create envs
rishabhsaxena Mar 19, 2020
6780780
Merge branch 'canary' into update-relay-modern-ssr-example
rishabhsaxena Mar 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 3 additions & 8 deletions examples/with-react-relay-network-modern/lib/createEnvironment/server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,10 @@ import {
urlMiddleware,
} from 'react-relay-network-modern/node8'
import RelaySSR from 'react-relay-network-modern-ssr/node8/server'
import { Network, Environment, RecordSource, Store } from 'relay-runtime'
import { Network, Environment } from 'relay-runtime'

export default {
initEnvironment: () => {
const source = new RecordSource()
const store = new Store(source)
initEnvironment: store => {
const relaySSR = new RelaySSR()

return {
Expand All @@ -24,10 +22,7 @@ export default {
}),
}
},
createEnvironment: (relayData, key) => {
const source = new RecordSource()
const store = new Store(source)

createEnvironment: (relayData, key, store) => {
return new Environment({
store,
network: Network.create(
Expand Down
18 changes: 15 additions & 3 deletions examples/with-react-relay-network-modern/pages/_app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,31 @@ import NextApp from 'next/app'

import { initEnvironment, createEnvironment } from '../lib/createEnvironment'

import { RecordSource, Store } from 'relay-runtime'

export default class App extends NextApp {
static getInitialProps = async ({ Component, router, ctx }) => {
let store

if (ctx.req) {
const source = new RecordSource()
store = new Store(source)
}

const { variables } = Component.getInitialProps
? await Component.getInitialProps(ctx)
: {}

try {
if (initEnvironment && Component.query) {
const { environment, relaySSR } = initEnvironment()
const { environment, relaySSR } = initEnvironment(store)

await fetchQuery(environment, Component.query, variables)

return {
variables,
relayData: await relaySSR.getCache(),
store,
}
}
} catch (e) {
Expand All @@ -27,17 +37,19 @@ export default class App extends NextApp {

return {
variables,
store,
Copy link
Member

Choose a reason for hiding this comment

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

Did you test that this works properly? only values that can be valid JSON are allowed to be returned in props, store here is a class

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah it seemed to work for me, needed a way to share the same store for initEnvironment and createEnvironment methods for SSR. We're not using the store prop at the client side.

Copy link
Member

Choose a reason for hiding this comment

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

@Timer Can you confirm that this will work properly 🙏

Copy link
Member

Choose a reason for hiding this comment

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

in my opinion you should create the store every time but send the state and hydrate the new store with the props returned in data fetching.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that seems a better option, if we're able to init the store using the relayData like we do for queryRecords in the with-relay-modern example

I was just wondering if QueryRenderer should provide the data before rendering, why isn't it fetching from the network in this case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does this seem better: rishabhsaxena@96b49f2 ?

Have used records to initate store as:

const records = environment
  .getStore()
  .getSource()
  .toJSON()

Did not need to use react-relay-network-modern-ssr for this though

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure, I don't know how Relay works, but once you update the example I can take a look into the code, test that it works and review it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lfades Ok sure, thanks. Have created another PR: #11343

}
}

render() {
const { Component, variables = {}, relayData } = this.props
const { Component, variables = {}, relayData, store } = this.props
const environment = createEnvironment(
relayData,
JSON.stringify({
queryID: Component.query ? Component.query.params.name : undefined,
variables,
})
}),
store
)

return (
Expand Down