Do not render disconnected nodes.

Author: jeswin

.vscode/launch.json

@@ -2,12 +2,10 @@
   "configurations": [
     {
       "args": [
-        "-r",
-        "esm",
         "--timeout",
         "999999",
         "--colors",
-        "${workspaceFolder}/test/dist/test"
+        "${workspaceFolder}/dist/test/test"
       ],
       "internalConsoleOptions": "openOnSessionStart",
       "name": "Mocha Tests",
@@ -17,7 +15,7 @@
         "<node_internals>/**"
       ],
       "type": "node",
-      "preLaunchTask": "build-all"
+      "preLaunchTask": "build-dev"
     }
   ]
 }

.vscode/tasks.json

@@ -1,26 +1,10 @@
 {
-	"version": "2.0.0",
-	"tasks": [
+  "version": "2.0.0",
+  "tasks": [
     {
       "type": "npm",
-      "script": "build",
-      "group": "build",
-      "problemMatcher": [],
-      "label": "npm: build",
-      "detail": "./build.sh"
-    },
-    {
-      "type": "npm",
-      "script": "build-test",
-      "group": "build",
-      "problemMatcher": [],
-      "label": "npm: build-test",
-      "detail": "(cd test && ./build.sh)",
-      "dependsOn": ["npm: build"]
-    },
-    {
-      "label": "build-all",
-      "dependsOn": ["npm: build", "npm: build-test"]
+      "label": "build-dev",
+      "script": "build-dev"
     }
   ]
-}
+}

package.json

@@ -1,6 +1,6 @@
 {
   "name": "forgo-state",
-  "version": "1.0.33",
+  "version": "1.0.34",
   "type": "module",
   "main": "./dist/index.js",
   "types": "./dist/index.d.ts",
@@ -26,6 +26,7 @@
   "scripts": {
     "clean": "rimraf ./dist",
     "build": "npm run clean && mkdir -p dist && npx tsc",
+    "build-dev": "npx tsc",
     "test": "mocha dist/test/test.js"
   },
   "license": "MIT"

src/index.ts

@@ -1,3 +1,11 @@
+/*
+  How it works:
+
+  - Users create JS proxies using the defineState() function.
+  - They bind this state (the proxy object) to various components via bindToStates() and bindToStateProps() functions.
+  - Since the proxy let's us capture changes to itself, we trigger component rerenders (on bound components) when that happens.
+*/
+
 import {
   ForgoRenderArgs,
   ForgoComponent,
@@ -8,58 +16,63 @@ import {
   NodeAttachedState,
 } from "forgo";
 
-export type ForgoProxyState = {};
-
-type StateMapEntry<TProps extends ForgoElementProps> = {
+type StateBoundComponentInfo<TProps extends ForgoElementProps> = {
   component: ForgoComponent<TProps>;
   args: ForgoRenderArgs;
 };
 
-type RerenderOnAnyChange<TState, TProps extends ForgoElementProps> = {
+type PropertyBoundComponentInfo<TState, TProps extends ForgoElementProps> = {
   propGetter: (state: TState) => any[];
-} & StateMapEntry<TProps>;
+} & StateBoundComponentInfo<TProps>;
 
-const stateMap: Map<any, StateMapEntry<any>[]> = new Map();
+const stateToComponentsMap: Map<any, StateBoundComponentInfo<any>[]> =
+  new Map();
 
 export function defineState<TState extends Record<string, any>>(
   state: TState
 ): TState {
   const handlers = {
     set(target: TState, prop: string & keyof TState, value: any) {
-      const entries = stateMap.get(proxy);
+      const entries = stateToComponentsMap.get(proxy);
 
       // if bound to the state directly, add for updation on any state change.
-      const argsForUncheckedUpdation: ForgoRenderArgs[] = entries
+      const stateBoundComponentArgs: ForgoRenderArgs[] = entries
         ? entries
-            .filter((x) => !(x as RerenderOnAnyChange<TState, any>).propGetter)
+            .filter(
+              (x) => !(x as PropertyBoundComponentInfo<TState, any>).propGetter
+            )
             .map((x) => x.args)
         : [];
 
-      // Get the props before update
-      let propsToCompare = entries
-        ? entries
-            .filter((x) => (x as RerenderOnAnyChange<TState, any>).propGetter)
-            .map((x) => ({
-              args: x.args,
-              props: (x as RerenderOnAnyChange<TState, any>).propGetter(target),
-            }))
+      const propBoundComponents = entries
+        ? entries.filter(
+            (x) => (x as PropertyBoundComponentInfo<TState, any>).propGetter
+          )
         : [];
 
+      // Get the props before update
+      let propBoundComponentArgs = propBoundComponents.map((x) => ({
+        args: x.args,
+        props: (x as PropertyBoundComponentInfo<TState, any>).propGetter(
+          target
+        ),
+      }));
+
       target[prop] = value;
 
       // Get the props after update
-      let updatedProps = entries
-        ? entries
-            .filter((x) => (x as RerenderOnAnyChange<TState, any>).propGetter)
-            .map((x) => ({
-              args: x.args,
-              props: (x as RerenderOnAnyChange<TState, any>).propGetter(target),
-            }))
-        : [];
-
-      // concat state based updates and props based updates
-      const argsListToUpdate = argsForUncheckedUpdation.concat(
-        propsToCompare
+      let updatedProps = propBoundComponents.map((x) => ({
+        args: x.args,
+        props: (x as PropertyBoundComponentInfo<TState, any>).propGetter(
+          target
+        ),
+      }));
+
+      // State bound components (a) need to be rerendered anyway.
+      // Prop bound components (b) are rendendered if changed.
+      // So concat (a) and (b)
+      const argsListToUpdate = stateBoundComponentArgs.concat(
+        propBoundComponentArgs
           .filter((oldProp, i) =>
             oldProp.props.some((p, j) => p !== updatedProps[i].props[j])
           )
@@ -84,7 +97,7 @@ export function defineState<TState extends Record<string, any>>(
       });
 
       // If a parent component is already rerendering,
-      //  don't queue the child rerender.
+      // don't queue the child rerender.
       const componentsToUpdate = componentStatesAndArgs.filter((item) => {
         const [componentState, args] = item;
 
@@ -141,7 +154,7 @@ let argsToRenderInTheNextCycle: ForgoRenderArgs[] = [];
 function doRender() {
   if (argsToRenderInTheNextCycle.length) {
     for (const args of argsToRenderInTheNextCycle) {
-      if (args.element.node) {
+      if (args.element.node && args.element.node.isConnected) {
         rerender(args.element);
       }
     }
@@ -167,23 +180,23 @@ export function bindToStateProps<TState, TProps extends ForgoElementProps>(
     ...component,
     mount(props: TProps, args: ForgoRenderArgs) {
       for (const [state, propGetter] of stateBindings) {
-        let entries = stateMap.get(state);
+        let entries = stateToComponentsMap.get(state);
 
         if (!entries) {
           entries = [];
-          stateMap.set(state, entries);
+          stateToComponentsMap.set(state, entries);
         }
 
         if (propGetter) {
-          const newEntry: RerenderOnAnyChange<TState, TProps> = {
+          const newEntry: PropertyBoundComponentInfo<TState, TProps> = {
             component: wrappedComponent,
             propGetter,
             args,
           };
 
           entries.push(newEntry);
         } else {
-          const newEntry: StateMapEntry<TProps> = {
+          const newEntry: StateBoundComponentInfo<TProps> = {
             component: wrappedComponent,
             args,
           };
@@ -198,10 +211,10 @@ export function bindToStateProps<TState, TProps extends ForgoElementProps>(
     },
     unmount(props: TProps, args: ForgoRenderArgs) {
       for (const [state] of stateBindings) {
-        let entry = stateMap.get(state);
+        let entry = stateToComponentsMap.get(state);
 
         if (entry) {
-          stateMap.set(
+          stateToComponentsMap.set(
             state,
             entry.filter((x) => x.component !== wrappedComponent)
           );

src/test/descendantMustNotRerender/index.ts

@@ -4,7 +4,7 @@ import { run } from "./script.js";
 import should from "should";
 
 export default function () {
-  it("skips rerendering descendant if parent is rerendering", async () => {
+  it("must not rerender descendent", async () => {
     const dom = new JSDOM(htmlFile(), {
       runScripts: "outside-only",
       resources: "usable",

src/test/dontRenderDisconnectedNodes/index.ts

@@ -0,0 +1,26 @@
+import { JSDOM } from "jsdom";
+import htmlFile from "../htmlFile.js";
+import { addNewMessage, renderAgain, run } from "./script.js";
+
+export default function () {
+  it("doesn't render disconnected nodes", async () => {
+    const dom = new JSDOM(htmlFile(), {
+      runScripts: "outside-only",
+      resources: "usable",
+    });
+    const window = dom.window;
+
+    run(dom);
+
+    await new Promise<void>((resolve) => {
+      window.addEventListener("load", () => {
+        resolve();
+      });
+    });
+
+    addNewMessage();
+    renderAgain();
+
+    window.document.body.innerHTML.trim().should.containEql(`<div id="root"></div>`)
+  });
+}

src/test/dontRenderDisconnectedNodes/script.tsx

@@ -0,0 +1,55 @@
+import * as forgo from "forgo";
+import { DOMWindow, JSDOM } from "jsdom";
+import { mount, ForgoRenderArgs, setCustomEnv } from "forgo";
+import { bindToStates, defineState } from "../../index.js";
+
+let window: DOMWindow;
+let document: Document;
+
+type State = {
+  messageCount: number;
+};
+
+const state: State = defineState({
+  messageCount: 0,
+});
+
+export function addNewMessage() {
+  state.messageCount++;
+}
+
+let counter = 0;
+
+let renderArgs: ForgoRenderArgs;
+export function renderAgain() {
+  counter++;
+  renderArgs.update();
+}
+
+function Parent() {
+  return {
+    render(props: {}, args: ForgoRenderArgs) {
+      renderArgs = args;
+      return counter === 0 ? <MessageBox /> : null;
+    },
+  };
+}
+
+function MessageBox() {
+  const component = {
+    render(props: any, args: ForgoRenderArgs) {
+      return <p>You have {state.messageCount} messages.</p>;
+    },
+  };
+  return bindToStates([state], component);
+}
+
+export function run(dom: JSDOM) {
+  window = dom.window;
+  document = window.document;
+  setCustomEnv({ window, document });
+
+  window.addEventListener("load", () => {
+    mount(<Parent />, document.getElementById("root"));
+  });
+}

src/test/test.ts

@@ -4,10 +4,12 @@ import bindToStateProps from "./bindToStateProps/index.js";
 import descendantMustNotRerender from "./descendantMustNotRerender/index.js";
 import batchesUpdates from "./batchesUpdates/index.js";
 import fragments from "./fragments/index.js";
+import dontRenderDisconnectedNodes from "./dontRenderDisconnectedNodes/index.js";
 
 defineState();
 bindToStates();
 bindToStateProps();
 descendantMustNotRerender();
 batchesUpdates();
 fragments();
+dontRenderDisconnectedNodes();