Skip to content

Commit

Permalink
(core) - Fix ssrExchange.restoreData adding invalidated results (#1776)
Browse files Browse the repository at this point in the history
* (core) - Fix ssrExchange.restoreData adding invalidated results

* Update tests with new case

* Fix type issues due to exposed SSR type
  • Loading branch information
kitten authored Jul 6, 2021
1 parent a0fcea4 commit 3547491
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 7 deletions.
5 changes: 5 additions & 0 deletions .changeset/cuddly-readers-walk.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@urql/core': patch
---

Prevent `ssrExchange().restoreData()` from adding results to the exchange that have already been invalidated. This may happen when `restoreData()` is called repeatedly, e.g. per page. When a prior run has already invalidated an SSR result then the result is 'migrated' to the user's `cacheExchange`, which means that `restoreData()` should never attempt to re-add it again.
33 changes: 32 additions & 1 deletion packages/core/src/exchanges/ssr.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ it('caches complex GraphQLErrors in query results correctly', () => {
publish(exchange);
next(queryOperation);

const error = ssr.extractData()[queryOperation.key].error;
const error = ssr.extractData()[queryOperation.key]!.error;

expect(error).toHaveProperty('graphQLErrors.0.message', 'Oh no!');
expect(error).toHaveProperty('graphQLErrors.0.path', ['Query']);
Expand Down Expand Up @@ -182,3 +182,34 @@ it('deletes cached results in non-suspense environments', async () => {
// NOTE: The operation should not be duplicated
expect(output).not.toHaveBeenCalled();
});

it('never allows restoration of invalidated results', async () => {
client.suspense = false;

const onPush = jest.fn();
const initialState = { [queryOperation.key]: serializedQueryResponse as any };

const ssr = ssrExchange({
isClient: true,
initialState: { ...initialState },
});

const { source: ops$, next } = input;
const exchange = ssr(exchangeInput)(ops$);

pipe(exchange, forEach(onPush));
next(queryOperation);

await Promise.resolve();

expect(Object.keys(ssr.extractData()).length).toBe(0);
expect(onPush).toHaveBeenCalledTimes(1);
expect(output).not.toHaveBeenCalled();

ssr.restoreData(initialState);
expect(Object.keys(ssr.extractData()).length).toBe(0);

next(queryOperation);
expect(onPush).toHaveBeenCalledTimes(2);
expect(output).toHaveBeenCalledTimes(1);
});
26 changes: 20 additions & 6 deletions packages/core/src/exchanges/ssr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ const deserializeResult = (

/** The ssrExchange can be created to capture data during SSR and also to rehydrate it on the client */
export const ssrExchange = (params?: SSRExchangeParams): SSRExchange => {
const data: SSRData = {};
const data: Record<string, SerializedResult | null> = {};

// On the client-side, we delete results from the cache as they're resolved
// this is delayed so that concurrent queries don't delete each other's data
Expand All @@ -101,13 +101,15 @@ export const ssrExchange = (params?: SSRExchangeParams): SSRExchange => {
if (invalidateQueue.length === 1) {
Promise.resolve().then(() => {
let key: number | void;
while ((key = invalidateQueue.shift())) delete data[key];
while ((key = invalidateQueue.shift())) {
data[key] = null;
}
});
}
};

const isCached = (operation: Operation) => {
return !shouldSkip(operation) && data[operation.key] !== undefined;
return !shouldSkip(operation) && data[operation.key] != null;
};

// The SSR Exchange is a temporary cache that can populate results into data for suspense
Expand All @@ -134,7 +136,7 @@ export const ssrExchange = (params?: SSRExchangeParams): SSRExchange => {
sharedOps$,
filter(op => isCached(op)),
map(op => {
const serialized = data[op.key];
const serialized = data[op.key]!;
return deserializeResult(op, serialized);
})
);
Expand All @@ -159,8 +161,20 @@ export const ssrExchange = (params?: SSRExchangeParams): SSRExchange => {
return merge([forwardedOps$, cachedOps$]);
};

ssr.restoreData = (restore: SSRData) => Object.assign(data, restore);
ssr.extractData = () => Object.assign({}, data);
ssr.restoreData = (restore: SSRData) => {
for (const key in restore) {
// We only restore data that hasn't been previously invalidated
if (data[key] !== null) {
data[key] = restore[key];
}
}
};

ssr.extractData = () => {
const result: SSRData = {};
for (const key in data) if (data[key] != null) result[key] = data[key]!;
return result;
};

if (params && params.initialState) {
ssr.restoreData(params.initialState);
Expand Down

0 comments on commit 3547491

Please sign in to comment.