Skip to content

Commit

Permalink
RFC: Only hold on to props that are fragment references (and not prop…
Browse files Browse the repository at this point in the history
…s from product)

Summary:
The purpose of this commit is to mitigate the impact of memory leaks produced by RelayModernFragmentSpecResolver when running in React Concurrent Mode.

Specifically, with the fragment spec resolver, we subscribe to the Relay store when a resolver is constructed, which occurs during render. Since in Concurrent Mode render can occur many times without ever committing, this leaves open the possibility of dangling resolvers with subscriptions that never get cleaned up.

This commit doesn't fix that specific issue, but makes it so we hold on to a lot less memory by not holding on to props passed to product code that aren't fragment references. Often times these props can reference parts of the React tree, causing us to hold on to a lot of memory that won't get cleaned up.

Note that the preferred way of running Relay in concurrent mode is to use Relay Hooks, which are concurrent safe.

Reviewed By: bgirard, steveluscher

Differential Revision: D18975901

fbshipit-source-id: bbb35bb1796509ce2ac3fce95b1bb3a87f35bff7
  • Loading branch information
Juan Tejada authored and facebook-github-bot committed Dec 18, 2019
1 parent 96f9ba8 commit b931d0b
Showing 1 changed file with 4 additions and 2 deletions.
Expand Up @@ -85,7 +85,7 @@ class RelayModernFragmentSpecResolver implements FragmentSpecResolver {
this._context = context;
this._data = {};
this._fragments = fragments;
this._props = props;
this._props = {};
this._resolvers = {};
this._stale = false;

Expand Down Expand Up @@ -138,6 +138,8 @@ class RelayModernFragmentSpecResolver implements FragmentSpecResolver {

setProps(props: Props): void {
const ownedSelectors = getSelectorsFromObject(this._fragments, props);
this._props = {};

for (const key in ownedSelectors) {
if (ownedSelectors.hasOwnProperty(key)) {
const ownedSelector = ownedSelectors[key];
Expand Down Expand Up @@ -178,10 +180,10 @@ class RelayModernFragmentSpecResolver implements FragmentSpecResolver {
resolver.setSelector(ownedSelector);
}
}
this._props[key] = props[key];
this._resolvers[key] = resolver;
}
}
this._props = props;
this._stale = true;
}

Expand Down

0 comments on commit b931d0b

Please sign in to comment.