Merge pull request #141 from opencomponents/fix-nested-polling

fix nested polling

Author: ricardo-devis-agullo

src/oc-client.js

@@ -518,41 +518,48 @@ export function createOc(oc) {
 			let isRendering = dataRendering == "true";
 			let isRendered = dataRendered == "true";
 
-			if (!isRendering && !isRendered) {
-				logInfo(MESSAGES_RETRIEVING);
-				setAttribute(dataRenderingAttribute, true);
-				if (!DISABLE_LOADER) {
-					component.innerHTML =
-						'<div class="oc-loading">' + MESSAGES_LOADING_COMPONENT + "</div>";
-				}
+			if (isRendered) {
+				callback();
+				return;
+			}
+			if (isRendering) {
+				timeout(() => {
+					oc.renderNestedComponent(component, callback);
+				}, POLLING_INTERVAL);
+				return;
+			}
 
-				oc.renderByHref(
-					{
-						href: getAttribute("href"),
-						id: getAttribute("id"),
-						element: component,
-					},
-					(err, data) => {
-						if (err || !data) {
-							setAttribute(dataRenderingAttribute, false);
-							setAttribute(dataRenderedAttribute, false);
-							setAttribute("data-failed", true);
-							component.innerHTML = "";
-							oc.events.fire("oc:failed", {
-								originalError: err,
-								data: data,
-								component,
-							});
-							logError(err);
-							callback();
-						} else {
-							processHtml(component, data, callback);
-						}
-					},
-				);
-			} else {
-				timeout(callback, POLLING_INTERVAL);
+			logInfo(MESSAGES_RETRIEVING);
+			setAttribute(dataRenderingAttribute, true);
+			if (!DISABLE_LOADER) {
+				component.innerHTML =
+					'<div class="oc-loading">' + MESSAGES_LOADING_COMPONENT + "</div>";
 			}
+
+			oc.renderByHref(
+				{
+					href: getAttribute("href"),
+					id: getAttribute("id"),
+					element: component,
+				},
+				(err, data) => {
+					if (err || !data) {
+						setAttribute(dataRenderingAttribute, false);
+						setAttribute(dataRenderedAttribute, false);
+						setAttribute("data-failed", true);
+						component.innerHTML = "";
+						oc.events.fire("oc:failed", {
+							originalError: err,
+							data: data,
+							component,
+						});
+						logError(err);
+						callback();
+					} else {
+						processHtml(component, data, callback);
+					}
+				},
+			);
 		});
 	};
 

tests/edgeCases.spec.js

@@ -169,7 +169,7 @@ test.describe("oc-client : edge cases", () => {
 		});
 
 		expect(result.callbackCalled).toBe(true);
-		expect(result.wasDelayed).toBe(true);
+		expect(result.wasDelayed).toBe(false);
 	});
 
 	test("should handle renderNestedComponent with currently rendering component", async ({
@@ -195,6 +195,11 @@ test.describe("oc-client : edge cases", () => {
 						wasDelayed: endTime - startTime >= 400,
 					});
 				});
+
+				setTimeout(() => {
+					element.setAttribute("data-rendering", "false");
+					element.setAttribute("data-rendered", "true");
+				}, 200);
 			});
 		});
 

tests/renderNestedComponent.spec.js

@@ -177,4 +177,86 @@ test.describe("oc-client : renderNestedComponent", () => {
 		expect(failureResult.failedEvent).toBeDefined();
 		expect(failureResult.failedEvent.component).toBeDefined();
 	});
+
+	test("should not re-render a component that has already been rendered", async ({
+		page,
+	}) => {
+		const result = await page.evaluate(
+			(config) => {
+				return new Promise((resolve) => {
+					// Create a DOM element
+					const component = document.createElement("oc-component");
+					component.setAttribute("href", config.componentHref);
+					component.setAttribute("data-rendered", "true");
+					component.innerHTML = "<div>previously rendered content</div>";
+					let renderByHrefCalls = 0;
+					// Mock renderByHref
+					oc.renderByHref = (href, cb) => {
+						renderByHrefCalls++;
+					};
+
+					// Call the function being tested
+					oc.renderNestedComponent(component, () => {
+						resolve({
+							innerHTML: component.innerHTML,
+							renderByHrefCalls: renderByHrefCalls,
+						});
+					});
+				});
+			},
+			{ componentHref },
+		);
+
+		// Verify the result
+		expect(result.innerHTML).toContain("previously rendered content");
+		expect(result.renderByHrefCalls).toBe(0);
+	});
+
+	test("should wait for a component that is already rendering", async ({
+		page,
+	}) => {
+		const result = await page.evaluate(
+			(config) => {
+				return new Promise((resolve) => {
+					const component = document.createElement("oc-component");
+					component.setAttribute("href", config.componentHref);
+					component.setAttribute("data-rendering", "true");
+					document.body.appendChild(component);
+
+					let renderByHrefCalls = 0;
+					oc.renderByHref = (href, cb) => {
+						renderByHrefCalls++;
+						cb(null, {
+							html: "<div>this is the component content</div>",
+							version: "1.0.0",
+							name: "my-component",
+							key: "12345678901234567890",
+						});
+					};
+
+					// Call the function being tested
+					oc.renderNestedComponent(component, () => {
+						const data = {
+							finalHtml: component.innerHTML,
+							renderByHrefCalls: renderByHrefCalls,
+						};
+						component.remove();
+						resolve(data);
+					});
+
+					// a bit later, we simulate the end of the rendering
+					setTimeout(() => {
+						component.setAttribute("data-rendering", "false");
+						component.setAttribute("data-rendered", "true");
+						component.innerHTML = "<div>newly rendered content</div>";
+					}, 200);
+				});
+			},
+			{ componentHref },
+		);
+
+		// Verify the results
+		expect(result.finalHtml).toContain("newly rendered content");
+		expect(result.renderByHrefCalls).toBe(0);
+	});
 });