From be2ae12462eef9c925db0950eb66413d28fdbbaf Mon Sep 17 00:00:00 2001 From: Jack Clark Date: Tue, 12 Mar 2019 09:45:38 +0000 Subject: [PATCH 1/2] fix: make cache required in ssr mode --- README.md | 2 +- packages/graphql-hooks-ssr/README.md | 2 +- packages/graphql-hooks-ssr/index.js | 7 +++++++ packages/graphql-hooks/src/GraphQLClient.js | 4 ++++ 4 files changed, 13 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 3b11c07b..5f8a7422 100644 --- a/README.md +++ b/README.md @@ -118,7 +118,7 @@ const client = new GraphQLClient(config) - `url` (**Required**): The url to your GraphQL server - `ssrMode`: Boolean - set to `true` when using on the server for server-side rendering; defaults to `false` -- `cache`: Object with the following methods: +- `cache` (**Required** if `ssrMode` is `true`, otherwise optional): Object with the following methods: - `cache.get(key)` - `cache.set(key, data)` - `cache.delete(key)` diff --git a/packages/graphql-hooks-ssr/README.md b/packages/graphql-hooks-ssr/README.md index fc61358b..728e8bc6 100644 --- a/packages/graphql-hooks-ssr/README.md +++ b/packages/graphql-hooks-ssr/README.md @@ -26,7 +26,7 @@ app.get('/', async (req, reply) => { // Step 1: Create the client inside the request handler const client = new GraphQLClient({ url: 'https://domain.com/graphql', - cache: memCache(), + cache: memCache() // NOTE: a cache is required for SSR, fetch }) diff --git a/packages/graphql-hooks-ssr/index.js b/packages/graphql-hooks-ssr/index.js index 51f94044..29725b3b 100644 --- a/packages/graphql-hooks-ssr/index.js +++ b/packages/graphql-hooks-ssr/index.js @@ -2,6 +2,13 @@ const ReactDOMServer = require('react-dom/server') async function getInitialState(opts) { const { App, client, render = ReactDOMServer.renderToStaticMarkup } = opts + + if (!client.cache) { + throw new Error( + 'A cache implementation must be provided for SSR, please pass one to `GraphQLClient` via `options`.' + ) + } + // ensure ssrMode is set: client.ssrMode = true render(App) diff --git a/packages/graphql-hooks/src/GraphQLClient.js b/packages/graphql-hooks/src/GraphQLClient.js index 3eae8891..f368dae3 100644 --- a/packages/graphql-hooks/src/GraphQLClient.js +++ b/packages/graphql-hooks/src/GraphQLClient.js @@ -15,6 +15,10 @@ class GraphQLClient { ) } + if (config.ssrMode && !config.cache) { + throw new Error('GraphQLClient: config.cache is required when in ssrMode') + } + this.cache = config.cache this.headers = config.headers || {} this.ssrMode = config.ssrMode From af0868d83cbb62834c631e91deb527bb5df2abdb Mon Sep 17 00:00:00 2001 From: Jack Clark Date: Tue, 12 Mar 2019 09:56:41 +0000 Subject: [PATCH 2/2] test: test that cache is required in ssrMode --- packages/graphql-hooks-ssr/index.test.js | 19 +++++++++++++++++++ .../test/unit/GraphQLClient.test.js | 17 +++++++++++++++-- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/packages/graphql-hooks-ssr/index.test.js b/packages/graphql-hooks-ssr/index.test.js index 105b9116..4a6bbe0b 100644 --- a/packages/graphql-hooks-ssr/index.test.js +++ b/packages/graphql-hooks-ssr/index.test.js @@ -27,4 +27,23 @@ describe('getInitialState', () => { expect(result).toEqual({ foo: 'bar' }) expect(resolvedPromises).toBe(2) }) + + it("throws if a cache hasn't been provided", async () => { + const MockApp = () => 'hello world' + + const mockClient = { + ssrPromises: [] + } + + expect( + getInitialState({ + App: MockApp, + client: mockClient + }) + ).rejects.toEqual( + new Error( + 'A cache implementation must be provided for SSR, please pass one to `GraphQLClient` via `options`.' + ) + ) + }) }) diff --git a/packages/graphql-hooks/test/unit/GraphQLClient.test.js b/packages/graphql-hooks/test/unit/GraphQLClient.test.js index a61d0242..186a5579 100644 --- a/packages/graphql-hooks/test/unit/GraphQLClient.test.js +++ b/packages/graphql-hooks/test/unit/GraphQLClient.test.js @@ -37,6 +37,15 @@ describe('GraphQLClient', () => { global.fetch = oldFetch }) + it('throws if config.ssrMode is true and no config.cache is provided', () => { + expect(() => { + new GraphQLClient({ + ...validConfig, + ssrMode: true + }) + }).toThrow('GraphQLClient: config.cache is required when in ssrMode') + }) + it('assigns config.cache to an instance property', () => { const cache = { get: 'get', set: 'set' } const client = new GraphQLClient({ ...validConfig, cache }) @@ -49,8 +58,12 @@ describe('GraphQLClient', () => { expect(client.headers).toBe(headers) }) - it('assigns config.ssrMode to an instance property', () => { - const client = new GraphQLClient({ ...validConfig, ssrMode: true }) + it('assigns config.ssrMode to an instance property if config.cache is provided', () => { + const client = new GraphQLClient({ + ...validConfig, + ssrMode: true, + cache: { get: 'get', set: 'set' } + }) expect(client.ssrMode).toBe(true) })