Skip to content

Commit

Permalink
feat: WeakMap when environments are referenced (#3014)
Browse files Browse the repository at this point in the history
Summary:
With this PR I aim to address the situation where environments are removed, but never garbage collected as references are held onto in this Map.

A simple case could be:

```js
const MyApp = () => {

	const [environment, setEnvironment] = useState(new Environment({ network, store }));

	useEffect(() => {
		setTimeout(() => {
			setEnvironment(new Environment({ network, store }));
		}, 1e3);
	}, []);

	return (<RelayEnvironmentProvider environment={environment}>
		<MyComponentTreeWith_A_useFragment/>
	</RelayEnvironmentProvider>);
};
```

as you'll find from that example, the `dataResources` map will be forever growing, with nothing clearing.

fixes: #3013
Pull Request resolved: #3014

Reviewed By: jstejada

Differential Revision: D19797050

Pulled By: alunyov

fbshipit-source-id: 4ef18e7634c7d50597944967e9ffc0a330ac3633
  • Loading branch information
maraisr authored and Juan Tejada committed Feb 13, 2020
1 parent 7611791 commit 4e08a86
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 3 deletions.
11 changes: 10 additions & 1 deletion packages/relay-experimental/FragmentResource.js
Expand Up @@ -40,6 +40,12 @@ export type FragmentResource = FragmentResourceImpl;

type FragmentResourceCache = Cache<Error | Promise<mixed> | FragmentResult>;

const WEAKMAP_SUPPORTED = typeof WeakMap === 'function';
interface IMap<K, V> {
get(key: K): V | void;
set(key: K, value: V): IMap<K, V>;
}

type SingularOrPluralSnapshot = Snapshot | $ReadOnlyArray<Snapshot>;
opaque type FragmentResult: {data: mixed, ...} = {|
cacheKey: string,
Expand Down Expand Up @@ -462,7 +468,10 @@ function createFragmentResource(environment: IEnvironment): FragmentResource {
return new FragmentResourceImpl(environment);
}

const dataResources: Map<IEnvironment, FragmentResource> = new Map();
const dataResources: IMap<IEnvironment, FragmentResource> = WEAKMAP_SUPPORTED
? new WeakMap()
: new Map();

function getFragmentResourceForEnvironment(
environment: IEnvironment,
): FragmentResourceImpl {
Expand Down
11 changes: 10 additions & 1 deletion packages/relay-experimental/QueryResource.js
Expand Up @@ -66,6 +66,12 @@ opaque type QueryResult: {
operation: OperationDescriptor,
|};

const WEAKMAP_SUPPORTED = typeof WeakMap === 'function';
interface IMap<K, V> {
get(key: K): V | void;
set(key: K, value: V): IMap<K, V>;
}

function getQueryCacheKey(
operation: OperationDescriptor,
fetchPolicy: FetchPolicy,
Expand Down Expand Up @@ -527,7 +533,10 @@ function createQueryResource(environment: IEnvironment): QueryResource {
return new QueryResourceImpl(environment);
}

const dataResources: Map<IEnvironment, QueryResource> = new Map();
const dataResources: IMap<IEnvironment, QueryResource> = WEAKMAP_SUPPORTED
? new WeakMap()
: new Map();

function getQueryResourceForEnvironment(
environment: IEnvironment,
): QueryResourceImpl {
Expand Down
6 changes: 5 additions & 1 deletion packages/relay-runtime/query/fetchQueryInternal.js
Expand Up @@ -34,7 +34,11 @@ type RequestCacheEntry = {|
+subscription: Subscription,
|};

const requestCachesByEnvironment = new Map();
const WEAKMAP_SUPPORTED = typeof WeakMap === 'function';

const requestCachesByEnvironment = WEAKMAP_SUPPORTED
? new WeakMap()
: new Map();

/**
* Fetches the given query and variables on the provided environment,
Expand Down

0 comments on commit 4e08a86

Please sign in to comment.