Repository: DevExpress/devextreme-react
Pull request #316 (2019-02-28 18:33:09)

Make TemplateUpdater overwrite re-added templates if imodel is the same
@nightskylark, here are the changes we discussed in regard to the #315 and #313.


Patch:
diff --git a/src/core/dx-template.ts b/src/core/dx-template.ts
index 320912e..6f90be8 100644
--- a/src/core/dx-template.ts
+++ b/src/core/dx-template.ts
@@ -21,16 +21,23 @@ function createDxTemplate(
     keyFn?: (data: any) => string
 ): IDxTemplate {
 
-    const renderedContainers: HTMLElement[] = [];
+    const renderedModels = new Map<any, string>();
     return {
         render: (data: IDxTemplateData) => {
-            const templateId = keyFn ? keyFn(data.model) : "__template_" + generateID();
-            const container = unwrapElement(data.container);
+            const prevTemplateId = renderedModels.get(data.model);
+
+            let templateId: string;
+            if (prevTemplateId) {
+                templateId = prevTemplateId;
+            } else {
+                templateId = keyFn ? keyFn(data.model) : "__template_" + generateID();
 
-            if (renderedContainers.indexOf(container) > -1) {
-                return container;
+                if (data.model !== undefined) {
+                    renderedModels.set(data.model, templateId);
+                }
             }
-            renderedContainers.push(container);
+
+            const container = unwrapElement(data.container);
 
             templateUpdater.setTemplate(templateId, () => {
                 const model = data.model;
@@ -43,7 +50,10 @@ function createDxTemplate(
                     {
                         content: contentProvider(model),
                         container,
-                        onRemoved: () => templateUpdater.removeTemplate(templateId),
+                        onRemoved: () => {
+                            templateUpdater.removeTemplate(templateId);
+                            renderedModels.delete(model);
+                        },
                         onRendered: data.onRendered,
                         key: templateId
                     }
diff --git a/src/core/template-updater.ts b/src/core/template-updater.ts
index 3542b65..07a671d 100644
--- a/src/core/template-updater.ts
+++ b/src/core/template-updater.ts
@@ -1,4 +1,4 @@
-import * as commonUtils from "devextreme/core/utils/common";
+import { deferUpdate } from "devextreme/core/utils/common";
 import { TemplateWrapperRenderer } from "./template-wrapper";
 
 interface ITemplateUpdater {
@@ -38,7 +38,7 @@ class TemplateUpdater implements ITemplateUpdater {
 
         this._updateIsDelayed = true;
 
-        commonUtils.deferUpdate(() => {
+        deferUpdate(() => {
             this._updateTemplatesCallback(this._templates);
             this._updateIsDelayed = false;
         });


Test patch:
diff --git a/src/core/__tests__/dx-template.test.tsx b/src/core/__tests__/dx-template.test.tsx
new file mode 100644
index 0000000..6b43da5
--- /dev/null
+++ b/src/core/__tests__/dx-template.test.tsx
@@ -0,0 +1,72 @@
+import { createDxTemplate } from "../dx-template";
+
+describe("dx-template", () => {
+
+    describe("template ID", () => {
+
+        it("is the same if template rendered twice for the same model", () => {
+            setTemplate.mockClear();
+            const model: any = {};
+
+            tryDoubleRender(model);
+
+            expect(setTemplate).toHaveBeenCalledTimes(2);
+
+            const firstId = setTemplate.mock.calls[0][0];
+            const secondId = setTemplate.mock.calls[1][0];
+            expect(secondId).toBe(firstId);
+        });
+
+        it("is the same if template rendered twice with null passed as model", () => {
+            // e.g. Lookup passes null as model if no item selected
+            setTemplate.mockClear();
+            const model: any = null;
+
+            tryDoubleRender(model);
+
+            expect(setTemplate).toHaveBeenCalledTimes(2);
+
+            const firstId = setTemplate.mock.calls[0][0];
+            const secondId = setTemplate.mock.calls[1][0];
+            expect(secondId).toBe(firstId);
+        });
+
+        it("is different if template rendered twice with undefined as model", () => {
+            // skip cases when model is undefined since no clear way of how to behave
+            setTemplate.mockClear();
+            const model: any = undefined;
+
+            tryDoubleRender(model);
+
+            expect(setTemplate).toHaveBeenCalledTimes(2);
+
+            const firstId = setTemplate.mock.calls[0][0];
+            const secondId = setTemplate.mock.calls[1][0];
+
+            expect(secondId).not.toBe(firstId);
+        });
+
+        const setTemplate = jest.fn();
+
+        function tryDoubleRender(model: any): void {
+            const template = createDxTemplate(
+                jest.fn(),
+                {
+                    setTemplate,
+                    removeTemplate: jest.fn()
+                }
+            );
+
+            template.render({
+                container: {},
+                model
+            });
+
+            template.render({
+                container: {},
+                model
+            });
+        }
+    });
+
+});
diff --git a/src/core/__tests__/template-dx-integration.test.tsx b/src/core/__tests__/template-dx-integration.test.tsx
deleted file mode 100644
index a9adca3..0000000
--- a/src/core/__tests__/template-dx-integration.test.tsx
+++ /dev/null
@@ -1,31 +0,0 @@
-import { createDxTemplate } from "../dx-template";
-
-describe("template-host", () => {
-
-    describe("its wrapped template", () => {
-
-        it("doesn't render twice into the same container", () => {
-            const setTemplate = jest.fn();
-            const container: any = {};
-            const template = createDxTemplate(
-                jest.fn(),
-                {
-                    setTemplate,
-                    removeTemplate: jest.fn()
-                },
-                jest.fn()
-            );
-
-            template.render({
-                container
-            });
-
-            template.render({
-                container
-            });
-
-            expect(setTemplate).toHaveBeenCalledTimes(1);
-        });
-    });
-
-});
diff --git a/src/core/__tests__/template-updater.test.tsx b/src/core/__tests__/template-updater.test.tsx
new file mode 100644
index 0000000..a04a01a
--- /dev/null
+++ b/src/core/__tests__/template-updater.test.tsx
@@ -0,0 +1,16 @@
+import { TemplateUpdater } from "../template-updater";
+
+describe("TemplateUpdater", () => {
+
+    it("overwrites re-added templates if templateId is the same", () => {
+        const updateFunc = jest.fn();
+        const templateUpdater = new TemplateUpdater(updateFunc);
+        const templateRenderer1 = jest.fn();
+        const templateRenderer2 = jest.fn();
+
+        templateUpdater.setTemplate("abc", templateRenderer1);
+        templateUpdater.setTemplate("abc", templateRenderer2);
+
+        expect(updateFunc).toHaveBeenLastCalledWith({ abc: templateRenderer2});
+    });
+});