Skip to content

Commit

Permalink
fix(core): fix possible memory leak on SSR and validate JSON on non-r…
Browse files Browse the repository at this point in the history
…aw request
  • Loading branch information
jackyef committed May 29, 2020
1 parent ef03bde commit eb196a2
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 8 deletions.
1 change: 1 addition & 0 deletions packages/react-isomorphic-data/src/common/Client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export const createDataClient = (
headers: headers || {},
fetchPolicy: fetchPolicy || 'cache-first',
toBePrefetched: {},
__internal: {},
addSubscriber: (key: string, callback: Function) => {
if (!(subscribers[key] instanceof Map)) {
subscribers[key] = new Map();
Expand Down
1 change: 1 addition & 0 deletions packages/react-isomorphic-data/src/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export interface DataClient {
fetchPolicy: 'cache-first' | 'cache-and-network' | 'network-only';
test: boolean;
headers: Record<string, any>;
__internal: Record<string, any>;
addSubscriber: (key: string, callback: Function) => void;
removeSubscriber: (key: string, callback: Function) => void;
notifySubscribers: (key: string) => void;
Expand Down
20 changes: 12 additions & 8 deletions packages/react-isomorphic-data/src/hooks/utils/useBaseData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import { LazyDataState, DataHookOptions } from '../types';
import useFetchRequirements from './useFetchRequirements';
import useCacheSubscription, { LoadingSymbol } from './useCacheSubscription';

const simpleCache: Record<string, any> = {};

const useBaseData = <T, > (
url: string,
queryParams: Record<string, any> = {},
Expand Down Expand Up @@ -48,7 +46,6 @@ const useBaseData = <T, > (
const delay = Date.now() - Number(client.initTime);

if (delay <= client.ssrForceFetchDelay) {
console.log('set to cache-first')
fetchPolicy = 'cache-first'; // force to 'cache-first' policy when using ssrForceFetchDelay
}
}
Expand All @@ -59,6 +56,9 @@ const useBaseData = <T, > (
promiseRef.current = fetcher(fullUrl, finalFetchOpts)
.then((result) => result.text())
.then((data) => {
if (!raw) {
JSON.parse(data); // check if valid JSON or not, will throw error if it is not
}
// this block of code will cause 2 re-renders because React doesn't batch these 2 updates
// https://twitter.com/dan_abramov/status/887963264335872000?lang=en
// For React 16.x we can use `unstable_batchedUpdates()` to solve this
Expand Down Expand Up @@ -105,7 +105,7 @@ const useBaseData = <T, > (
});

return promiseRef.current;
}, [addToCache, finalFetchOpts, fullUrl, isSSR, useTempData, fetcher, setState]);
}, [addToCache, finalFetchOpts, fullUrl, isSSR, useTempData, fetcher, setState, raw]);

const memoizedFetchData = React.useCallback((): Promise<any> => {
const currentDataInCache = retrieveFromCache(fullUrl);
Expand Down Expand Up @@ -190,13 +190,17 @@ const useBaseData = <T, > (
return usedData;
} else {
// else, we want the data in json form
if (!simpleCache[usedData]) {
simpleCache[usedData] = JSON.parse(usedData);
if (!client.__internal[usedData]) {
try {
client.__internal[usedData] = JSON.parse(usedData);
} catch (err) {
client.__internal[usedData] = null;
}
}

return simpleCache[usedData];
return client.__internal[usedData];
}
}, [usedData, raw]);
}, [usedData, raw, client]);

return [
memoizedFetchData,
Expand Down

0 comments on commit eb196a2

Please sign in to comment.