Skip to content

Commit

Permalink
Remove maxSize option (defaults internally to 50 now), do not invalid…
Browse files Browse the repository at this point in the history
…ate when offline or browser window is inactive
  • Loading branch information
vlad-zhukov committed Nov 19, 2019
1 parent ac1eeb1 commit fa88816
Show file tree
Hide file tree
Showing 6 changed files with 227 additions and 33 deletions.
1 change: 0 additions & 1 deletion priem.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ export declare interface Options {
}

export declare interface ResourceOptions {
maxSize?: number;
ssrKey?: string;
}

Expand Down
76 changes: 61 additions & 15 deletions src/Resource.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import is, {TypeName} from '@sindresorhus/is';
import {assertType, isBrowser, shallowEqual} from './utils';
import {assertType, browserActivityState, isBrowser, shallowEqual} from './utils';
import {Cache, CacheItem, SerializableCacheItem, reduce} from './Cache';

const DEFAULT_MAX_SIZE = 50;
Expand Down Expand Up @@ -40,6 +40,43 @@ export function toSerializableArray(
});
}

const scheduledTimers = new Map<number, () => void>();

browserActivityState.subscribe(() => {
if (browserActivityState.isActive()) {
scheduledTimers.forEach(handler => handler());
scheduledTimers.clear();
}
});

function setCacheItemTimeout(item: CacheItem<unknown, unknown>, handler: () => void, timeout: number): void {
/* istanbul ignore else */
if (isBrowser) {
/* istanbul ignore if */
if (item.expireTimerId) {
window.clearTimeout(item.expireTimerId);
scheduledTimers.delete(item.expireTimerId);
}
const timerId = window.setTimeout(() => {
if (!browserActivityState.isActive() && timerId) {
scheduledTimers.set(timerId, handler);
} else {
handler();
}
item.expireTimerId = undefined;
}, timeout);
item.expireTimerId = timerId;
}
}

function clearCacheItemTimeout(item: CacheItem<unknown, unknown>): void {
if (isBrowser && item.expireTimerId) {
window.clearTimeout(item.expireTimerId);
scheduledTimers.delete(item.expireTimerId);
item.expireTimerId = undefined;
}
}

// Only used during SSR for resources with `ssrKey`
const resourceList = new Set<Resource<unknown, any>>();

Expand Down Expand Up @@ -192,12 +229,16 @@ export class Resource<DataType, Args extends Record<string, unknown>> {

if (shouldUpdate) {
this.update(item, maxAge);
} else if (!item.expireTimerId && isBrowser && maxAge) {
item.expireTimerId = window.setTimeout(() => {
if (item && item.key) {
this.invalidate(item.key);
}
}, maxAge);
} else if (!item.expireTimerId && maxAge) {
setCacheItemTimeout(
item,
() => {
if (item && item.key) {
this.invalidate(item.key);
}
},
maxAge,
);
}

return item.value;
Expand All @@ -217,7 +258,7 @@ export class Resource<DataType, Args extends Record<string, unknown>> {
const timesUsed = this.onCacheChange(args, shouldCommit);

if (timesUsed > 0) {
window.clearTimeout(item.expireTimerId);
clearCacheItemTimeout(item);
item.isValid = false; // This will trigger an update
} else {
this.cache.remove(item);
Expand All @@ -228,6 +269,7 @@ export class Resource<DataType, Args extends Record<string, unknown>> {

/** @private */
update(item: CacheItem<Args, MemoizedValue<DataType>>, maxAge?: number): void {
clearCacheItemTimeout(item);
Object.assign(item.value, {status: Status.PENDING, data: undefined, reason: undefined});

const promise = this.fn(item.key)
Expand All @@ -242,13 +284,17 @@ export class Resource<DataType, Args extends Record<string, unknown>> {

const timesUsed = this.onCacheChange(item.key);

if (timesUsed > 0 && isBrowser && maxAge) {
window.clearTimeout(item.expireTimerId);
item.expireTimerId = window.setTimeout(() => {
if (item.key) {
this.invalidate(item.key);
}
}, maxAge);
if (timesUsed > 0 && maxAge) {
setCacheItemTimeout(
item,
() => {
/* istanbul ignore else */
if (item.key) {
this.invalidate(item.key);
}
},
maxAge,
);
}
})
.catch(error => {
Expand Down
4 changes: 2 additions & 2 deletions src/__tests__/Resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ it('should invalidate', async () => {
expect(onCacheChange).toHaveBeenCalledTimes(3);
});

it('should not exceed the default max size of 50', async () => {
it('should not exceed the default max size of 50', () => {
const resource = new Resource<number, {value: number}>(({value}) => delay(200, {value}));

for (let i = 0; i < 50; i++) {
Expand Down Expand Up @@ -279,7 +279,7 @@ it('should not exceed the default max size of 50', async () => {
expect(tail).toMatchInlineSnapshot(`CacheItem {}`);

// `tail` was removed from the cache
expect(resource['cache'].findBy(item => shallowEqual(item ,{value: 0}))).toBe(undefined);
expect(resource['cache'].findBy(item => shallowEqual(item, {value: 0}))).toBe(undefined);
});

it('should guard if promise resolves after item was removed', async () => {
Expand Down
112 changes: 99 additions & 13 deletions src/__tests__/createResource.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,55 @@ const readSpy = jest.spyOn(Resource.prototype, 'read');
const updateSpy = jest.spyOn(Resource.prototype, 'update');
const onCacheChangeSpy = jest.spyOn(Resource.prototype, 'onCacheChange');

let timers: number[] = [];
let navigatorOnline = window.navigator.onLine;
Object.defineProperty(window.navigator, 'onLine', {
get() {
return navigatorOnline;
},
});

function goOffline() {
navigatorOnline = false;
window.dispatchEvent(new window.Event('offline'));
}

function goOnline() {
navigatorOnline = true;
window.dispatchEvent(new window.Event('online'));
}

let documentHidden = window.document.hidden;
Object.defineProperty(window.document, 'hidden', {
get() {
return documentHidden;
},
});

function hideWindow() {
documentHidden = true;
window.dispatchEvent(new window.Event('visibilitychange'));
}

function showWindow() {
documentHidden = false;
window.dispatchEvent(new window.Event('visibilitychange'));
}

let timerIds: number[] = [];
const originalSetTimeout = window.setTimeout;
// eslint-disable-next-line @typescript-eslint/ban-ts-ignore
// @ts-ignore
window.setTimeout = (handler, timeout, ...args) => {
timers.push(originalSetTimeout(handler, timeout, ...args));
const timerId = originalSetTimeout(handler, timeout, ...args);
timerIds.push(timerId);
return timerId;
};

afterEach(() => {
timers.filter(window.clearTimeout);
timers = [];
timerIds.filter(window.clearTimeout);
timerIds = [];
navigatorOnline = true;
documentHidden = false;
readSpy.mockClear();
updateSpy.mockClear();
onCacheChangeSpy.mockClear();
Expand Down Expand Up @@ -142,18 +180,18 @@ it('should rerun promises when cache expires if `maxAge` is set', async () => {
// expire (pending)

expect(getLastReturn()).toBe('foo2');
expect(useResourceSpy).toHaveBeenCalledTimes(6);
expect(useResourceSpy).toHaveBeenCalledTimes(5);
expect(updateSpy).toHaveBeenCalledTimes(3);
expect(onCacheChangeSpy).toHaveBeenCalledTimes(5);
expect(onCacheChangeSpy).toHaveBeenCalledTimes(4);

await act(() => delay(200));

// fulfilled

expect(getLastReturn()).toBe('foo2');
expect(useResourceSpy).toHaveBeenCalledTimes(7);
expect(useResourceSpy).toHaveBeenCalledTimes(6);
expect(updateSpy).toHaveBeenCalledTimes(3);
expect(onCacheChangeSpy).toHaveBeenCalledTimes(6);
expect(onCacheChangeSpy).toHaveBeenCalledTimes(5);
});

it('should have `invalidate` method', async () => {
Expand Down Expand Up @@ -384,10 +422,10 @@ it('should debounce calls', async () => {
return ret;
});

const Comp: React.FC<{arg: string}> = props => {
function Comp(props: {arg: string}) {
useResourceSpy({value: props.arg});
return null;
};
}

const {rerender} = render(<Comp arg="foo" />);
rerender(<Comp arg="bar" />);
Expand All @@ -413,7 +451,7 @@ it('should debounce calls', async () => {
rejected: false,
},
]);
expect(readSpy).toHaveBeenCalledTimes(3);
expect(readSpy).toHaveBeenCalledTimes(2);

await act(() => delay(200));

Expand All @@ -426,7 +464,7 @@ it('should debounce calls', async () => {
rejected: false,
},
]);
expect(readSpy).toHaveBeenCalledTimes(4);
expect(readSpy).toHaveBeenCalledTimes(3);

await act(() => delay(300));

Expand All @@ -439,7 +477,7 @@ it('should debounce calls', async () => {
rejected: false,
},
]);
expect(readSpy).toHaveBeenCalledTimes(4);
expect(readSpy).toHaveBeenCalledTimes(3);
});

it('should invalidate on mount when `refreshOnMount` is set', async () => {
Expand Down Expand Up @@ -489,6 +527,54 @@ it('should invalidate on mount when `refreshOnMount` is set', async () => {
]);
});

it('should schedule updates when browser is offline', async () => {
const useResource = createResource<string, {}>(() => delay(100, {value: 'foo'}));
const useResourceSpy = jest.fn(useResource);

function Comp() {
useResourceSpy({}, {maxAge: 500});
return null;
}

render(<Comp />);
expect(useResourceSpy).toHaveBeenCalledTimes(1);
await act(() => delay(100));
expect(useResourceSpy).toHaveBeenCalledTimes(2);

act(() => goOffline());
await act(() => delay(700));
expect(useResourceSpy).toHaveBeenCalledTimes(2);

act(() => goOnline());
expect(useResourceSpy).toHaveBeenCalledTimes(3);
await act(() => delay(100));
expect(useResourceSpy).toHaveBeenCalledTimes(4);
});

it('should schedule updates when browser tab is not active', async () => {
const useResource = createResource<string, {}>(() => delay(100, {value: 'foo'}));
const useResourceSpy = jest.fn(useResource);

function Comp() {
useResourceSpy({}, {maxAge: 500});
return null;
}

render(<Comp />);
expect(useResourceSpy).toHaveBeenCalledTimes(1);
await act(() => delay(100));
expect(useResourceSpy).toHaveBeenCalledTimes(2);

act(() => hideWindow());
await act(() => delay(700));
expect(useResourceSpy).toHaveBeenCalledTimes(2);

act(() => showWindow());
expect(useResourceSpy).toHaveBeenCalledTimes(3);
await act(() => delay(100));
expect(useResourceSpy).toHaveBeenCalledTimes(4);
});

it('should hydrate data', async () => {
hydrateStore([
[
Expand Down
8 changes: 6 additions & 2 deletions src/createResource.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as React from 'react';
import {TypeName} from '@sindresorhus/is';
import {Resource, ResourceOptions, Subscriber, Status, MemoizedKey} from './Resource';
import {assertType, shallowEqual, useForceUpdate, useLazyRef} from './utils';
import {assertType, isBrowser, shallowEqual, useForceUpdate, useLazyRef} from './utils';

const DEFAULT_DEBOUNCE_MS = 150;

Expand Down Expand Up @@ -85,7 +85,11 @@ export function createResource<DataType, Args extends MemoizedKey>(
* 4. The item is not in the cache.
*/
const shouldDebounce =
args !== undefined && !!prevResult && now - lastTimeCalled < DEFAULT_DEBOUNCE_MS && !resource.has(args);
isBrowser &&
args !== undefined &&
!!prevResult &&
now - lastTimeCalled < DEFAULT_DEBOUNCE_MS &&
!resource.has(args);

// TODO: rework debounce
React.useEffect(() => {
Expand Down
Loading

0 comments on commit fa88816

Please sign in to comment.