From b931d0b1961cd04580d7f019ed3a0be9818f1ea6 Mon Sep 17 00:00:00 2001 From: Juan Tejada Date: Tue, 17 Dec 2019 18:28:14 -0800 Subject: [PATCH] RFC: Only hold on to props that are fragment references (and not props 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 --- .../relay-runtime/store/RelayModernFragmentSpecResolver.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/relay-runtime/store/RelayModernFragmentSpecResolver.js b/packages/relay-runtime/store/RelayModernFragmentSpecResolver.js index 7c3bdabbfe26c..c389dd9e247d5 100644 --- a/packages/relay-runtime/store/RelayModernFragmentSpecResolver.js +++ b/packages/relay-runtime/store/RelayModernFragmentSpecResolver.js @@ -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; @@ -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]; @@ -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; }