Repository: DevExpress/devextreme-react
Pull request #400 (2019-12-18 09:44:46)

Rework templates events listening


Patch:
diff --git a/src/core/template-wrapper.ts b/src/core/template-wrapper.ts
index 78656a0..c982b23 100644
--- a/src/core/template-wrapper.ts
+++ b/src/core/template-wrapper.ts
@@ -12,28 +12,40 @@ interface ITemplateWrapperProps {
     key: string;
 }
 
+interface ITemplateWrapperState {
+    removalListenerRequired: boolean;
+}
+
 type TemplateWrapperRenderer = () => TemplateWrapper;
 
-class TemplateWrapper extends React.PureComponent<ITemplateWrapperProps> {
+const removalListenerStyle = { display: "none" };
+
+class TemplateWrapper extends React.PureComponent<ITemplateWrapperProps, ITemplateWrapperState> {
     private readonly _removalListenerRef = React.createRef<HTMLElement>();
 
+    private get _listenerElement(): HTMLElement {
+        return this._removalListenerRef.current as HTMLElement;
+    }
+
     constructor(props: ITemplateWrapperProps) {
         super(props);
+
+        this.state = { removalListenerRequired: false };
+
+        this._onDxRemove = this._onDxRemove.bind(this);
     }
 
     public render() {
+        const removalListener = this.state.removalListenerRequired
+            ? React.createElement("span", { style: removalListenerStyle, ref: this._removalListenerRef })
+            : undefined;
+
         return ReactDOM.createPortal(
             React.createElement(
                 React.Fragment,
                 null,
                 this.props.content,
-                React.createElement(
-                    this.props.container.nodeName === "TABLE" ? "tbody" : "span",
-                    {
-                        style: { display: "none" },
-                        ref: this._removalListenerRef
-                    }
-                )
+                removalListener
             ),
             this.props.container
         );
@@ -59,20 +71,36 @@ class TemplateWrapper extends React.PureComponent<ITemplateWrapperProps> {
         if (node) {
             this.props.container.appendChild(node);
         }
-        this.props.container.appendChild(
-            this._removalListenerRef.current as HTMLElement
-        );
+        if (this._listenerElement) {
+            this.props.container.appendChild(this._listenerElement);
+        }
     }
 
     private _subscribeOnRemove() {
-        const removalListener = this._removalListenerRef.current;
-        if (!removalListener) {
-            // T713245 (ref to removalListener is undefined under certain conditions)
+        const node = ReactDOM.findDOMNode(this);
+
+        if (node && node.nodeType === Node.ELEMENT_NODE) {
+            this._subscribeOnElementRemoval(node as Element);
             return;
         }
-        events.one(removalListener, DX_REMOVE_EVENT, () => {
-            this.props.onRemoved();
-        });
+
+        if (!this.state.removalListenerRequired) {
+            this.setState({ removalListenerRequired: true });
+            return;
+        }
+
+        if (this._listenerElement) {
+            this._subscribeOnElementRemoval(this._listenerElement);
+        }
+    }
+
+    private _subscribeOnElementRemoval(element: Element): void {
+        events.off(element, DX_REMOVE_EVENT, this._onDxRemove);
+        events.one(element, DX_REMOVE_EVENT, this._onDxRemove);
+    }
+
+    private _onDxRemove(): void {
+        this.props.onRemoved();
     }
 }
 


Test patch:
diff --git a/src/core/__tests__/template.test.tsx b/src/core/__tests__/template.test.tsx
index df17f47..98fca04 100644
--- a/src/core/__tests__/template.test.tsx
+++ b/src/core/__tests__/template.test.tsx
@@ -98,7 +98,7 @@ function testTemplateOption(testedOption: string) {
         component.update();
 
         expect(component.html())
-            .toBe("<div>Text<div><div>Template</div><span style=\"display: none;\"></span></div></div>");
+            .toBe("<div>Text<div><div>Template</div></div></div>");
     });
 
     it("renders new template after component change", () => {
@@ -143,16 +143,16 @@ function testTemplateOption(testedOption: string) {
 
     it("renders template removeEvent listener", () => {
         const elementOptions: Record<string, any> = {};
-        elementOptions[testedOption] = prepareTemplate((data: any) => <div>Template {data.text}</div>);
+        elementOptions[testedOption] = prepareTemplate((data: any) => <>Template {data.text}</>);
         const component = mount(React.createElement(ComponentWithTemplates, elementOptions));
 
         const container = document.createElement("div");
         renderItemTemplate({ text: "with data"}, container);
         component.update();
-        expect(container.innerHTML).toBe('<div>Template with data</div><span style=\"display: none;\"></span>');
+        expect(container.innerHTML).toBe('Template with data<span style=\"display: none;\"></span>');
     });
 
-    it("renders template removeEvent listener for table", () => {
+    it("does not render template removeEvent listener", () => {
         const elementOptions: Record<string, any> = {};
         elementOptions[testedOption] = prepareTemplate((data: any) => (
             <tbody><tr><td>Template {data.text}</td></tr></tbody>
@@ -164,7 +164,7 @@ function testTemplateOption(testedOption: string) {
 
         component.update();
         expect(container.innerHTML)
-            .toBe("<tbody><tr><td>Template with data</td></tr></tbody><tbody style=\"display: none;\"></tbody>");
+            .toBe("<tbody><tr><td>Template with data</td></tr></tbody>");
     });
 
     it("calls onRendered callback", () => {
@@ -224,14 +224,39 @@ function testTemplateOption(testedOption: string) {
         expect(templatesKeys[1]).toBe("2");
     });
 
-    it("removes deleted templates", () => {
+    it("removes text template", () => {
         const elementOptions: Record<string, any> = {};
         elementOptions[testedOption] = prepareTemplate((data: any) => (
-            <div className={"template"}>Template {data.text}</div>
+            <>Template {data.text}</>
         ));
         const component = mount(React.createElement(ComponentWithTemplates, elementOptions));
         const componentInstance = component.instance() as any;
 
+        const container = document.createElement("div");
+        renderItemTemplate({}, container);
+        expect(componentInstance._templatesStore.renderWrappers().length).toBe(1);
+        component.update();
+        const removeListener = container.getElementsByTagName("SPAN")[0];
+
+        const parentElement = removeListener.parentElement;
+        if (!parentElement) { throw new Error(); }
+
+        parentElement.innerHTML = "";
+        events.triggerHandler(removeListener, "dxremove");
+        component.update();
+        expect(componentInstance._templatesStore.renderWrappers().length).toBe(0);
+    });
+
+    it("removes template", () => {
+        const elementOptions: Record<string, any> = {
+            [testedOption]: prepareTemplate((data: any) => (
+                <div className={"template"}>Template {data.text}</div>
+            ))
+        };
+
+        const component = mount(React.createElement(ComponentWithTemplates, elementOptions));
+        const componentInstance = component.instance() as any;
+
         renderItemTemplate();
         expect(componentInstance._templatesStore.renderWrappers().length).toBe(1);
         component.update();
@@ -240,11 +265,8 @@ function testTemplateOption(testedOption: string) {
         const parentElement = templateContent.parentElement;
         if (!parentElement) { throw new Error(); }
 
-        const removeListener = parentElement.getElementsByTagName("SPAN")[0];
-
-        parentElement.removeChild(removeListener);
         parentElement.removeChild(templateContent);
-        events.triggerHandler(removeListener, "dxremove");
+        events.triggerHandler(templateContent, "dxremove");
         component.update();
         expect(componentInstance._templatesStore.renderWrappers().length).toBe(0);
     });