@samitouri / QOS-React-2 / commits / c3be085910

Remove crawling for updates

It doesn't seem necessary to crawl because mounts only happen in the context of updates. This fixes updates to not be treated as new mounts.

Dan Abramov committed May 31, 2019 at 17:40 UTC c3be0859100f065c3b94b3cf43d764cc8a79e05a
2 files changed +85 -82
src/backend/legacy/renderer.js
+84 -79
@@ -149,15 +149,17 @@ export function attach(
149 renderer.Mount,
150 '_renderNewRootComponent',
151 internalInstance => {
152 - const id = getID(internalInstance);
152 + // TODO: we might need to reset currentParentID before this runs.
153
154 + const id = getID(internalInstance);
155 rootIDs.add(id);
156
157 if (__DEBUG__) {
158 console.log('renderer.Mount._renderNewRootComponent()', id);
159 }
160
160 - recordPendingMount(internalInstance);
161 + // TODO: maybe we need to record this mount.
162 + // Needs testing.
163
164 // If we're mounting a root, we've just finished a batch of work,
165 // so it's safe to synchronously flush.
@@ -171,15 +173,17 @@ export function attach(
173 renderer.Mount,
174 'renderComponent',
175 internalInstance => {
174 - const id = getID(internalInstance);
176 + // TODO: we might need to reset currentParentID before this runs.
177
178 + const id = getID(internalInstance);
179 rootIDs.add(id);
180
181 if (__DEBUG__) {
182 console.log('renderer.Mount.renderComponent()', id);
183 }
184
182 - recordPendingMount(internalInstance);
185 + // TODO: maybe we need to record this mount.
186 + // Needs testing.
187
188 // If we're mounting a root, we've just finished a batch of work,
189 // so it's safe to synchronously flush.
@@ -188,24 +192,50 @@ export function attach(
192 );
193 }
194
195 + // This is shared mutable state that lets us keep track of where we are.
196 + let currentParentID = 0;
197 +
198 if (renderer.Reconciler) {
199 oldReconcilerMethods = decorateMany(renderer.Reconciler, {
193 - mountComponent(internalInstance, rootID, transaction, context) {
200 + mountComponent(fn, args) {
201 + const [internalInstance] = args;
202 +
203 recordPendingMount(internalInstance);
204 +
205 + let prevParentID = currentParentID;
206 + currentParentID = getID(internalInstance);
207 + const result = fn.apply(this, args);
208 + currentParentID = prevParentID;
209 +
210 + return result;
211 },
196 - performUpdateIfNecessary(
197 - internalInstance,
198 - nextChild,
199 - transaction,
200 - context
201 - ) {
202 - // TODO Check for change in order of children
212 + performUpdateIfNecessary(fn, args) {
213 + const [internalInstance] = args;
214 +
215 + let prevParentID = currentParentID;
216 + currentParentID = getID(internalInstance);
217 + const result = fn.apply(this, args);
218 + currentParentID = prevParentID;
219 +
220 + return result;
221 },
204 - receiveComponent(internalInstance, nextChild, transaction, context) {
205 - // TODO Check for change in order of children
222 + receiveComponent(fn, args) {
223 + const [internalInstance] = args;
224 +
225 + let prevParentID = currentParentID;
226 + currentParentID = getID(internalInstance);
227 + const result = fn.apply(this, args);
228 + currentParentID = prevParentID;
229 +
230 + return result;
231 },
207 - unmountComponent(internalInstance) {
232 + unmountComponent(fn, args) {
233 + const [internalInstance] = args;
234 +
235 + const result = fn.apply(this, args);
236 +
237 recordPendingUnmount(internalInstance);
238 + return result;
239 },
240 });
241 }
@@ -229,7 +259,6 @@ export function attach(
259 oldRenderComponent = null;
260 }
261
232 - const mountedIDs: Set<number> = new Set();
262 const pendingMountIDs: Set<number> = new Set();
263 const pendingUnmountIDs: Set<number> = new Set();
264 const pendingOperations: Array<number> = [];
@@ -295,57 +324,29 @@ export function attach(
324
325 rootIDs.add(id);
326
298 - crawlAndRecordMounts(id, 0, true);
327 + crawlAndRecordInitialMounts(id, 0);
328
329 // It's safe to synchronously flush for the root we just crawled.
330 flushPendingEvents(id);
331 }
332 }
333
305 - function crawlAndRecordMounts(
306 - id: number,
307 - parentID: number,
308 - isInitialMount: boolean
309 - ) {
334 + // TODO: this isn't covered by tests.
335 + // Might be broken.
336 + function crawlAndRecordInitialMounts(id: number, parentID: number) {
337 const internalInstance = idToInternalInstanceMap.get(id);
338
312 - const shouldIncludeInTree =
313 - parentID === 0 ||
314 - getElementType(internalInstance) !== ElementTypeOtherOrUnknown;
315 -
339 // Not all nodes are mounted in the frontend DevTools tree,
340 // but it's important to track parent info even for the unmounted ones.
341 idToParentIDMap.set(id, parentID);
342
343 if (__DEBUG__) {
321 - console.group(
322 - 'crawlAndRecordMounts() id:',
323 - id,
324 - 'shouldIncludeInTree?',
325 - shouldIncludeInTree
326 - );
327 - }
328 -
329 - if (shouldIncludeInTree) {
330 - const didMount = isInitialMount || pendingMountIDs.has(id);
331 - const didUnmount = pendingUnmountIDs.has(id);
332 -
333 - // If this node was both mounted and unmounted in the same batch,
334 - // just skip it and don't send any update.
335 - if (didMount && didUnmount) {
336 - pendingUnmountIDs.delete(id);
337 - return;
338 - } else if (didMount) {
339 - recordMount(id, parentID);
340 - }
344 + console.group('crawlAndRecordInitialMounts() id:', id);
345 }
346
347 + recordMount(id, parentID);
348 getChildIDs(internalInstance).forEach(childID =>
344 - crawlAndRecordMounts(
345 - childID,
346 - shouldIncludeInTree ? id : parentID,
347 - isInitialMount
348 - )
349 + crawlAndRecordInitialMounts(childID, id)
350 );
351
352 if (__DEBUG__) {
@@ -354,38 +355,42 @@ export function attach(
355 }
356
357 function flushPendingEvents(rootID: number): void {
357 - // Crawl tree and record mounts/updates.
358 - crawlAndRecordMounts(rootID, 0, false);
359 -
358 // Record pending deletions.
359 const unmountIDs = [];
360 pendingUnmountIDs.forEach(id => {
363 - if (mountedIDs.has(id)) {
364 - const internalInstance = idToInternalInstanceMap.get(id);
365 - const isRoot = rootIDs.has(id);
366 -
367 - if (__DEBUG__) {
368 - console.log(
369 - '%crecordUnmount()',
370 - 'color: red; font-weight: bold;',
371 - id,
372 - getData(internalInstance).displayName
373 - );
374 - }
375 -
376 - if (isRoot) {
377 - pendingUnmountedRootID = id;
361 + const internalInstance = idToInternalInstanceMap.get(id);
362 + const isRoot = rootIDs.has(id);
363 +
364 + if (__DEBUG__) {
365 + console.log(
366 + '%crecordUnmount()',
367 + 'color: red; font-weight: bold;',
368 + id,
369 + getData(internalInstance).displayName
370 + );
371 + }
372
379 - rootIDs.delete(id);
380 - } else {
381 - unmountIDs.push(id);
382 - }
373 + // TODO: handle the case where it was never mounted.
374 + if (isRoot) {
375 + pendingUnmountedRootID = id;
376 + rootIDs.delete(id);
377 + } else {
378 + unmountIDs.push(id);
379 + }
380
384 - idToInternalInstanceMap.delete(id);
385 - internalInstanceToIDMap.delete(internalInstance);
381 + idToInternalInstanceMap.delete(id);
382 + internalInstanceToIDMap.delete(internalInstance);
383 + });
384
387 - mountedIDs.delete(id);
385 + pendingMountIDs.forEach(id => {
386 + if (pendingUnmountIDs.has(id)) {
387 + return;
388 }
389 + const parentID = idToParentIDMap.get(id);
390 + if (parentID === undefined) {
391 + return;
392 + }
393 + recordMount(id, parentID);
394 });
395
396 const numUnmountIDs =
@@ -632,7 +637,9 @@ export function attach(
637 }
638
639 function recordPendingMount(internalInstance: InternalInstance) {
635 - pendingMountIDs.add(getID(internalInstance));
640 + const id = getID(internalInstance);
641 + pendingMountIDs.add(id);
642 + idToParentIDMap.set(id, currentParentID);
643
644 if (__DEBUG__) {
645 console.log(
@@ -680,8 +687,6 @@ export function attach(
687 );
688 }
689
683 - mountedIDs.add(id);
684 -
690 if (isRoot) {
691 // TODO Is this right? For all versions?
692 const hasOwnerMetadata =
src/backend/legacy/utils.js
+1 -3
@@ -19,9 +19,7 @@ export function decorateResult(
19 export function decorate(object: Object, attr: string, fn: Function): Function {
20 const old = object[attr];
21 object[attr] = function(instance: InternalInstance) {
22 - const res = old.apply(this, arguments);
23 - fn.apply(this, arguments);
24 - return res;
22 + return fn.call(this, old, arguments);
23 };
24 return old;
25 }