diff --git a/packages/react-testing/CHANGELOG.md b/packages/react-testing/CHANGELOG.md index 049d2e67af..01071977ab 100644 --- a/packages/react-testing/CHANGELOG.md +++ b/packages/react-testing/CHANGELOG.md @@ -5,6 +5,10 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). +## [Unreleased] + +- Improved the performance of Root wrapper updates [#1812](https://github.com/Shopify/quilt/pull/1812) + ## [2.2.2] - 2021-03-03 ### Fixed diff --git a/packages/react-testing/src/element.ts b/packages/react-testing/src/element.ts index 63e9854b10..e80f0bbf4e 100644 --- a/packages/react-testing/src/element.ts +++ b/packages/react-testing/src/element.ts @@ -45,6 +45,24 @@ export class Element implements Node { return this.elementChildren; } + private get elementDescendants() { + if (!this.descendantsCache) { + this.descendantsCache = getDescendants(this); + } + + return this.descendantsCache; + } + + private get elementChildren() { + if (!this.elementChildrenCache) { + this.elementChildrenCache = this.allChildren.filter( + element => typeof element !== 'string', + ) as Element[]; + } + + return this.elementChildrenCache; + } + get descendants() { return this.elementDescendants; } @@ -71,23 +89,14 @@ export class Element implements Node { return domNodes[0] || null; } - private readonly elementChildren: Element[]; - private readonly elementDescendants: Element[]; + private descendantsCache: Element[] | null = null; + private elementChildrenCache: Element[] | null = null; constructor( private readonly tree: Tree, private readonly allChildren: (Element | string)[], - allDescendants: (Element | string)[], public readonly root: Root, - ) { - this.elementChildren = allChildren.filter( - element => typeof element !== 'string', - ) as Element[]; - - this.elementDescendants = allDescendants.filter( - element => typeof element !== 'string', - ) as Element[]; - } + ) {} data(key: string): string | undefined { return this.props[key.startsWith('data-') ? key : `data-${key}`]; @@ -252,3 +261,18 @@ function equalSubset(subset: object, full: object) { key => key in full && full[key] === subset[key], ); } + +function getDescendants(element: any) { + const descendants: Element[] = []; + // eslint-disable-next-line @typescript-eslint/prefer-for-of + for (let i = 0; i < element.allChildren.length; i++) { + const child = element.allChildren[i]; + if (typeof child !== 'string') { + descendants.push(child); + // eslint-disable-next-line prefer-spread + descendants.push.apply(descendants, child.elementDescendants); + } + } + + return descendants; +} diff --git a/packages/react-testing/src/root.tsx b/packages/react-testing/src/root.tsx index dd861e8278..3a5e7ef479 100644 --- a/packages/react-testing/src/root.tsx +++ b/packages/react-testing/src/root.tsx @@ -253,10 +253,12 @@ export class Root implements Node { if (this.wrapper == null) { this.root = null; } else { - const topElement = flatten( - ((this.wrapper as unknown) as ReactInstance)._reactInternalFiber, + const topElement = fiberToElement( + findCurrentFiberUsingSlowPath( + ((this.wrapper as unknown) as ReactInstance)._reactInternalFiber, + ), this, - )[0]; + ); this.root = this.resolveRoot(topElement as any) as any; } @@ -284,52 +286,40 @@ function defaultRender(element: React.ReactElement) { return element; } -function flatten( - element: Fiber, +function fiberToElement( + node: Fiber, root: Root, -): (Element | string)[] { - const node: Fiber = findCurrentFiberUsingSlowPath(element); - +): Element | string { if (node.tag === Tag.HostText) { - return [node.memoizedProps as string]; - } - - const props = {...((node.memoizedProps as any) || {})}; - const {children, descendants} = childrenToTree(node.child, root); - - return [ - new Element( - { - tag: node.tag, - type: node.type, - props, - instance: node.stateNode, - }, - children, - descendants, - root, - ), - ...descendants, - ]; + return node.memoizedProps as string; + } + + const props = node.memoizedProps as any; + const children = childrenToTree(node.child, root); + + return new Element( + { + tag: node.tag, + type: node.type, + props, + instance: node.stateNode, + }, + children, + root, + ); } function childrenToTree(fiber: Fiber | null, root: Root) { let currentFiber = fiber; const children: (string | Element)[] = []; - const descendants: (string | Element)[] = []; while (currentFiber != null) { - const result = flatten(currentFiber, root); - - if (result.length > 0) { - children.push(result[0]); - descendants.push(...result); - } - + const result = fiberToElement(currentFiber, root); + children.push(result); currentFiber = currentFiber.sibling; } - return {children, descendants}; + return children; } function isPromise(promise: T | Promise): promise is Promise { diff --git a/packages/react-testing/src/tests/element.test.tsx b/packages/react-testing/src/tests/element.test.tsx index 1f8202fc77..074b864d93 100644 --- a/packages/react-testing/src/tests/element.test.tsx +++ b/packages/react-testing/src/tests/element.test.tsx @@ -15,8 +15,7 @@ const defaultTree = { }; const defaultRoot = new Root(); - -const divOne = new Element( +const divTwo = new Element( { ...defaultTree, type: 'div', @@ -24,33 +23,29 @@ const divOne = new Element( instance: document.createElement('div'), }, [], - [], defaultRoot, ); -const divTwo = new Element( +const divOne = new Element( { ...defaultTree, type: 'div', tag: Tag.HostComponent, instance: document.createElement('div'), }, - [], - [], + [divTwo], defaultRoot, ); -const componentOne = new Element( +const componentTwo = new Element( {...defaultTree, type: DummyComponent}, [], - [], defaultRoot, ); -const componentTwo = new Element( +const componentOne = new Element( {...defaultTree, type: DummyComponent}, - [], - [], + [divOne, componentTwo], defaultRoot, ); @@ -58,7 +53,7 @@ describe('Element', () => { describe('#props', () => { it('returns the props from the tree', () => { const props = {foo: 'bar'}; - const element = new Element({...defaultTree, props}, [], [], defaultRoot); + const element = new Element({...defaultTree, props}, [], defaultRoot); expect(element).toHaveProperty('props', props); }); }); @@ -66,7 +61,7 @@ describe('Element', () => { describe('#type', () => { it('returns the type from the tree', () => { const type = DummyComponent; - const element = new Element({...defaultTree, type}, [], [], defaultRoot); + const element = new Element({...defaultTree, type}, [], defaultRoot); expect(element).toHaveProperty('type', type); }); }); @@ -74,12 +69,7 @@ describe('Element', () => { describe('#instance', () => { it('returns the type from the tree', () => { const instance = {}; - const element = new Element( - {...defaultTree, instance}, - [], - [], - defaultRoot, - ); + const element = new Element({...defaultTree, instance}, [], defaultRoot); expect(element).toHaveProperty('instance', instance); }); }); @@ -89,7 +79,6 @@ describe('Element', () => { const element = new Element( {...defaultTree, tag: Tag.MemoComponent}, [], - [], defaultRoot, ); expect(element).toHaveProperty('isDOM', false); @@ -99,7 +88,6 @@ describe('Element', () => { const element = new Element( {...defaultTree, tag: Tag.HostComponent}, [], - [], defaultRoot, ); expect(element).toHaveProperty('isDOM', true); @@ -108,12 +96,7 @@ describe('Element', () => { describe('#children', () => { it('returns element children', () => { - const element = new Element( - defaultTree, - [divOne, divTwo], - [divOne, divTwo], - defaultRoot, - ); + const element = new Element(defaultTree, [divOne, divTwo], defaultRoot); expect(element).toHaveProperty('children', [divOne, divTwo]); }); @@ -122,7 +105,6 @@ describe('Element', () => { const element = new Element( defaultTree, [divOne, 'Some text', divTwo], - [divOne, 'Some text', divTwo], defaultRoot, ); @@ -132,23 +114,7 @@ describe('Element', () => { describe('#descendants', () => { it('returns element descendants', () => { - const element = new Element( - defaultTree, - [divOne], - [divOne, divTwo], - defaultRoot, - ); - - expect(element).toHaveProperty('descendants', [divOne, divTwo]); - }); - - it('does not return string descendants', () => { - const element = new Element( - defaultTree, - [divOne], - [divOne, 'Some text', divTwo], - defaultRoot, - ); + const element = new Element(defaultTree, [divOne], defaultRoot); expect(element).toHaveProperty('descendants', [divOne, divTwo]); }); @@ -160,12 +126,7 @@ describe('Element', () => { }); it('returns the instances associated with each child DOM element', () => { - const element = new Element( - defaultTree, - [divOne, divTwo], - [divOne, divTwo], - defaultRoot, - ); + const element = new Element(defaultTree, [divOne, divTwo], defaultRoot); expect(element.domNodes).toStrictEqual([ divOne.instance, @@ -174,23 +135,13 @@ describe('Element', () => { }); it('does not return descendant DOM nodes', () => { - const element = new Element( - defaultTree, - [divOne], - [divOne, divTwo], - defaultRoot, - ); + const element = new Element(defaultTree, [divOne], defaultRoot); expect(element.domNodes).not.toContain(divTwo.instance); }); it('does not return instances for non-DOM nodes', () => { - const element = new Element( - defaultTree, - [componentOne], - [componentOne], - defaultRoot, - ); + const element = new Element(defaultTree, [componentOne], defaultRoot); expect(element.domNodes).not.toContain(componentOne.instance); }); @@ -202,34 +153,19 @@ describe('Element', () => { }); it('returns null if there is no direct child DOM node', () => { - const element = new Element( - defaultTree, - [componentOne], - [componentOne], - defaultRoot, - ); + const element = new Element(defaultTree, [componentOne], defaultRoot); expect(element.domNode).toBeNull(); }); it('returns the DOM node if there is a single DOM child', () => { - const element = new Element( - defaultTree, - [divOne], - [divOne, divTwo], - defaultRoot, - ); + const element = new Element(defaultTree, [divOne], defaultRoot); expect(element.domNode).toBe(divOne.instance); }); it('throws an error if there are multiple top-level DOM nodes', () => { - const element = new Element( - defaultTree, - [divOne, divTwo], - [divOne, divTwo], - defaultRoot, - ); + const element = new Element(defaultTree, [divOne, divTwo], defaultRoot); expect(() => element.domNode).toThrow(/multiple HTML elements/); }); @@ -238,7 +174,7 @@ describe('Element', () => { describe('#prop()', () => { it('returns the prop value for the specified key', () => { const props = {foo: 'bar'}; - const element = new Element({...defaultTree, props}, [], [], defaultRoot); + const element = new Element({...defaultTree, props}, [], defaultRoot); expect(element.prop('foo')).toBe(props.foo); }); }); @@ -253,7 +189,6 @@ describe('Element', () => { const element = new Element( {...defaultTree, tag: Tag.HostComponent, type: 'div', instance: div}, [], - [], defaultRoot, ); @@ -265,13 +200,12 @@ describe('Element', () => { const childTextTwo = 'bar'; const descendantText = ' baz?'; - const elementChild = new Element(defaultTree, [], [], defaultRoot); + const elementChild = new Element(defaultTree, [], defaultRoot); jest.spyOn(elementChild, 'text').mockImplementation(() => childTextOne); const element = new Element( {...defaultTree, tag: Tag.FunctionComponent, type: DummyComponent}, [elementChild, childTextTwo], - [elementChild, childTextTwo, descendantText], defaultRoot, ); @@ -282,7 +216,6 @@ describe('Element', () => { const element = new Element( {...defaultTree, tag: Tag.HostPortal, type: DummyComponent}, ['Hello world'], - ['Hello world'], defaultRoot, ); @@ -300,7 +233,6 @@ describe('Element', () => { const element = new Element( {...defaultTree, tag: Tag.HostComponent, type: 'div', instance: div}, [], - [], defaultRoot, ); @@ -310,15 +242,13 @@ describe('Element', () => { it('concatenates the HTML contents of all child elements and child text', () => { const childHtml = 'foo '; const childText = 'bar'; - const descendantText = ' baz?'; - const elementChild = new Element(defaultTree, [], [], defaultRoot); + const elementChild = new Element(defaultTree, [], defaultRoot); jest.spyOn(elementChild, 'text').mockImplementation(() => childHtml); const element = new Element( {...defaultTree, tag: Tag.FunctionComponent, type: DummyComponent}, [elementChild, childText], - [elementChild, childText, descendantText], defaultRoot, ); @@ -329,7 +259,6 @@ describe('Element', () => { const element = new Element( {...defaultTree, tag: Tag.HostPortal, type: DummyComponent}, ['Hello world'], - ['Hello world'], defaultRoot, ); @@ -342,7 +271,6 @@ describe('Element', () => { const element = new Element( {...defaultTree, tag: Tag.HostComponent, type: 'div'}, [], - [], defaultRoot, ); @@ -353,7 +281,6 @@ describe('Element', () => { const element = new Element( {...defaultTree, tag: Tag.FunctionComponent, type: DummyComponent}, [], - [], defaultRoot, ); @@ -363,12 +290,7 @@ describe('Element', () => { describe('#find()', () => { it('finds the first matching DOM node', () => { - const element = new Element( - defaultTree, - [componentOne], - [componentOne, divOne, divTwo], - defaultRoot, - ); + const element = new Element(defaultTree, [componentOne], defaultRoot); expect(element.find('div')).toBe(divOne); }); @@ -376,8 +298,7 @@ describe('Element', () => { it('finds the first matching component', () => { const element = new Element( defaultTree, - [divOne], - [divOne, componentOne, componentTwo], + [divOne, componentOne], defaultRoot, ); @@ -394,7 +315,6 @@ describe('Element', () => { instance: document.createElement('div'), }, [], - [], defaultRoot, ); @@ -407,7 +327,6 @@ describe('Element', () => { instance: document.createElement('div'), }, [], - [], defaultRoot, ); @@ -420,14 +339,12 @@ describe('Element', () => { instance: document.createElement('span'), }, [], - [], defaultRoot, ); const element = new Element( defaultTree, [divOne, divTwo, span], - [divOne, divTwo, span], defaultRoot, ); @@ -439,30 +356,20 @@ describe('Element', () => { }); it('returns null when no match is found', () => { - const element = new Element(defaultTree, [], [], defaultRoot); + const element = new Element(defaultTree, [], defaultRoot); expect(element.find(DummyComponent)).toBeNull(); }); }); describe('#findAll()', () => { it('finds all matching DOM nodes', () => { - const element = new Element( - defaultTree, - [divOne], - [divOne, componentOne, divTwo], - defaultRoot, - ); + const element = new Element(defaultTree, [divOne], defaultRoot); expect(element.findAll('div')).toStrictEqual([divOne, divTwo]); }); it('finds all matching components', () => { - const element = new Element( - defaultTree, - [componentOne], - [componentOne, divOne, componentTwo], - defaultRoot, - ); + const element = new Element(defaultTree, [componentOne], defaultRoot); expect(element.findAll(DummyComponent)).toStrictEqual([ componentOne, @@ -480,7 +387,6 @@ describe('Element', () => { instance: document.createElement('div'), }, [], - [], defaultRoot, ); @@ -493,7 +399,6 @@ describe('Element', () => { instance: document.createElement('div'), }, [], - [], defaultRoot, ); @@ -506,14 +411,12 @@ describe('Element', () => { instance: document.createElement('span'), }, [], - [], defaultRoot, ); const element = new Element( defaultTree, [divOne, divTwo, span], - [divOne, divTwo, span], defaultRoot, ); @@ -527,7 +430,7 @@ describe('Element', () => { }); it('returns an empty array when no matches are found', () => { - const element = new Element(defaultTree, [], [], defaultRoot); + const element = new Element(defaultTree, [], defaultRoot); expect(element.findAll(DummyComponent)).toHaveLength(0); }); }); @@ -538,12 +441,7 @@ describe('Element', () => { (element: Element) => element === divTwo, ); - const element = new Element( - defaultTree, - [componentOne], - [componentOne, divOne, divTwo], - defaultRoot, - ); + const element = new Element(defaultTree, [componentOne], defaultRoot); expect(element.findWhere(matches)).toBe(divTwo); expect(matches).toHaveBeenCalledWith(componentOne); @@ -552,24 +450,14 @@ describe('Element', () => { }); it('returns null when no match is found', () => { - const element = new Element( - defaultTree, - [componentOne], - [componentOne], - defaultRoot, - ); + const element = new Element(defaultTree, [componentOne], defaultRoot); expect(element.findWhere(() => false)).toBeNull(); }); }); describe('#findAllWhere()', () => { it('finds all matching nodes', () => { - const element = new Element( - defaultTree, - [divOne], - [divOne, componentOne, divTwo], - defaultRoot, - ); + const element = new Element(defaultTree, [divOne], defaultRoot); expect( element.findAllWhere(element => element === componentTwo), @@ -594,7 +482,6 @@ describe('Element', () => { const element = new Element( {...defaultTree, type: TriggerableComponent, props: {}}, [], - [], defaultRoot, ); @@ -608,7 +495,6 @@ describe('Element', () => { const element = new Element( {...defaultTree, type: TriggerableComponent, props: {onClick}}, [], - [], defaultRoot, ); @@ -626,7 +512,6 @@ describe('Element', () => { props: {onClick: () => ''}, }, [], - [], defaultRoot, ); @@ -643,7 +528,6 @@ describe('Element', () => { props: {onClick: () => returnValue}, }, [], - [], defaultRoot, ); @@ -659,7 +543,6 @@ describe('Element', () => { props: {onClick: jest.fn()}, }, [], - [], defaultRoot, ); @@ -688,7 +571,6 @@ describe('Element', () => { props: {actions: []}, }, [], - [], defaultRoot, ); @@ -705,7 +587,6 @@ describe('Element', () => { props: {actions: []}, }, [], - [], defaultRoot, ); @@ -725,7 +606,6 @@ describe('Element', () => { props: {actions: [{onAction}]}, }, [], - [], defaultRoot, ); @@ -743,7 +623,6 @@ describe('Element', () => { props: {actions: [{onAction: () => returnValue}]}, }, [], - [], defaultRoot, ); diff --git a/packages/react-testing/src/tests/root.test.tsx b/packages/react-testing/src/tests/root.test.tsx index c0d2baaad8..e4e0b17759 100644 --- a/packages/react-testing/src/tests/root.test.tsx +++ b/packages/react-testing/src/tests/root.test.tsx @@ -23,7 +23,6 @@ describe('Root', () => { instance: childInstance, }, [], - [], root, ); @@ -40,7 +39,6 @@ describe('Root', () => { instance, }, [childElement], - [childElement], root, );