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.

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

(core) - Fix ssrExchange.restoreData adding invalidated results #1776

Merged
merged 3 commits into from
Jul 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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