ENGWORK-5595: Make MVC compatible with React 18

config/build.js

@@ -1,6 +1,10 @@
 import { build } from 'esbuild';
 import { dtsPlugin } from 'esbuild-plugin-d.ts';
 
+// TODO: It might be easier to replace this with `tsdx`, which makes it easy to build both ESM and
+// CJS output with correct .d.ts files without worrying about externals. Using "exports" in
+// package.json helps tools find the right files they need.
+
 build({
   entryPoints: ['src/index.ts'],
   bundle: true,
@@ -11,10 +15,8 @@ build({
   external: [
     'react',
     'react-dom',
-    '@aha-app/react-easy-state',
     '@nx-js/observer-util',
     'debug',
-    'lodash',
   ],
   target: 'es2018', // TODO: remove this when aha-app supports esnext.
   platform: 'browser',

package.json

@@ -8,6 +8,7 @@
   "files": [
     "dist"
   ],
+  "sideEffects": false,
   "scripts": {
     "build": "rimraf dist && node config/build.js && tsc",
     "prepare": "yarn build",
@@ -24,20 +25,18 @@
   "homepage": "https://github.com/aha-app/mvc#readme",
   "dependencies": {
     "@aha-app/react-easy-state": "^0.0.12-development",
-    "debug": "^4.1.0",
-    "lodash": "^4.17.21 || npm:lodash-es@^4.17.21"
+    "debug": "^4.1.0"
   },
   "peerDependencies": {
-    "react": "^16.8.4",
-    "react-dom": "^16.8.4"
+    "react": "^16.8.4 || ^17.0.0 || ^18.3.1",
+    "react-dom": "^16.8.4 || ^17.0.0 || ^18.3.1"
   },
   "devDependencies": {
     "@testing-library/jest-dom": "^6.2.0",
     "@testing-library/react": "^14.1.2",
     "@testing-library/user-event": "^14.5.2",
     "@types/debug": "^4.1.12",
     "@types/jest": "^29.5.11",
-    "@types/lodash": "^4.14.202",
     "@types/react": "^18.2.48",
     "esbuild": "^0.19.11",
     "esbuild-plugin-d.ts": "^1.1.0",
@@ -49,5 +48,6 @@
     "rimraf": "^5.0.0",
     "ts-jest": "^29.1.1",
     "typescript": "^5.0.4"
-  }
+  },
+  "packageManager": "yarn@1.22.22"
 }

src/controller/ApplicationController.tsx

@@ -3,15 +3,16 @@ import React, {
   Ref,
   useContext,
   useEffect,
+  useRef,
   useState,
 } from 'react';
 import type { ComponentType, FC, ReactNode } from 'react';
 // @ts-ignore
-import { store } from '@aha-app/react-easy-state';
 import Debug from 'debug';
 import { randomId } from '../utils/randomId';
-import { cloneDeep } from 'lodash';
-import { observe, unobserve } from '..';
+import { observe, unobserve } from '@nx-js/observer-util';
+import { shallowEqual } from '../utils/shallowEqual';
+import { store } from '../store/Store';
 
 const debug = Debug('framework:controller');
 
@@ -41,9 +42,8 @@ type GetControllerProps<T extends ApplicationControllerConstructor<any>> =
  * 3. The `state` object, and any content within it, must only be mutated
  *    inside an `action...` function.
  * 4. Action functions can be called from anywhere, including event handlers,
- *    callbacks, after `await`, render methods, and from within other action
- *    functions.
- *
+ *    callbacks, after `await`, and from within other action
+ *    functions. They should NOT be called from a component render.
  */
 class ApplicationController<
   State extends {} = {},
@@ -52,9 +52,8 @@ class ApplicationController<
 > {
   id: string;
   initialized: boolean;
-  parent: Parent;
-  state: State;
-  proxiedThis: any;
+  parent: Parent | null;
+  state: State | undefined;
   _debug = Debug(`controller:${this.constructor.name}`);
   runOnDestroy: Array<() => void>;
 
@@ -67,7 +66,7 @@ class ApplicationController<
     this.state = undefined;
     this.runOnDestroy = [];
 
-    this.proxiedThis = new Proxy(this, {
+    const proxiedThis = new Proxy(this, {
       // Traverse up through the controller hierarchy and find one that responds
       // to the specified action.
       get(targetController, prop, receiver) {
@@ -107,7 +106,7 @@ class ApplicationController<
       },
     });
 
-    return this.proxiedThis;
+    return proxiedThis;
   }
 
   /**
@@ -116,14 +115,14 @@ class ApplicationController<
    *
    * @abstract
    */
-  async initialize(props: Props): Promise<void> {}
+  initialize(props: Props): void | Promise<void> {}
 
   /**
    * Internal initializer function
    *
    * @hidden
    */
-  internalInitialize(parentController: Parent, initialArgs: Props) {
+  private internalInitialize(parentController: Parent, initialArgs: Props) {
     if (!this.initialized) {
       this.parent = parentController;
 
@@ -133,21 +132,21 @@ class ApplicationController<
         }`
       );
 
-      // @ts-ignore props are readonly, as we don't want them reassigned, but we need to set them here
-      this.props = store({ ...initialArgs });
+      // props are readonly, as we don't want them reassigned, but we need to set them here
+      (this.props as any) = store({ ...initialArgs });
 
-      this.state = store(cloneDeep(this.initialState));
+      this.state = store(structuredClone(this.initialState));
       if (this.initialize) this.initialize(initialArgs);
       this.initialized = true;
     } else {
-      const oldProps = { ...this.props };
-      Object.keys(initialArgs).forEach(key => {
-        if (this.props[key] !== initialArgs[key]) {
-          this.props[key] = initialArgs[key];
-        }
-      });
-
-      this.changeProps(initialArgs, oldProps);
+      // const oldProps = { ...this.props };
+      // Object.keys(initialArgs).forEach(key => {
+      //   if (this.props[key] !== initialArgs[key]) {
+      //     this.props[key] = initialArgs[key];
+      //   }
+      // });
+
+      // this.changeProps(initialArgs, oldProps);
     }
   }
 
@@ -169,9 +168,8 @@ class ApplicationController<
 
   /**
    * Internal destroy function. Do not override
-   * @private
    */
-  internalDestroy() {
+  private internalDestroy() {
     this.destroy();
     this.runOnDestroy.forEach(fn => fn());
   }
@@ -312,7 +310,7 @@ function StartControllerScope<
   // allocate a new controller instance.
   return React.memo((controllerInitialArgs: any) => {
     // eslint-disable-next-line react-hooks/rules-of-hooks
-    const [controller] = useState(new ControllerClass());
+    const [controller] = useState(() => new ControllerClass());
 
     if (controllerInitialArgs?.controllerRef) {
       if (typeof controllerInitialArgs.controllerRef === 'function') {
@@ -343,10 +341,10 @@ function StartControllerScope<
 export const ControllerContext = React.createContext(null);
 
 /**
- * A component that initializes a controller instance and wraps its
- * child with a context containing that instance.
+ * A component that initializes a controller instance, connects its lifecycle methods to React, and
+ * wraps its child with a context containing that instance.
  */
-function Controller<Props = {}>({
+function Controller<Props extends {} = {}>({
   children,
   controller,
   controllerInitialArgs,
@@ -357,10 +355,23 @@ function Controller<Props = {}>({
 }) {
   const parentController = useContext(ControllerContext);
 
+  // TODO: only initialize once, even with strict mode double renders
   controller.internalInitialize(parentController, controllerInitialArgs);
 
+  // inform the controller of a change in props. Should be done in an effect, a render may not be
+  // committed.
+  const prevPropsRef = useRef<Props>(controllerInitialArgs);
+  useEffect(() => {
+    const prevProps = prevPropsRef.current;
+    if (!shallowEqual(prevProps, controllerInitialArgs)) {
+      controller.changeProps(controllerInitialArgs, prevProps);
+    }
+    prevPropsRef.current = controllerInitialArgs;
+  }, [controllerInitialArgs]);
+
   // Give controller a chance to deregister when it is removed.
   useEffect(() => {
+    // TODO: Strict mode/concurrent features break this, must be symmetric
     return () => {
       debug('Destroying controller');
       controller.internalDestroy();

src/index.ts

@@ -4,20 +4,9 @@ import {
   ControlledComponent,
   useController,
 } from './controller/ApplicationController';
-// @ts-ignore
-import { view } from '@aha-app/react-easy-state';
 import { raw, observable, observe, unobserve } from '@nx-js/observer-util';
 import { randomId } from './utils/randomId';
-import type { ComponentType } from 'react';
-
-function ApplicationView<C extends ComponentType>(component: C): C;
-function ApplicationView<T extends {}>(
-  component: ComponentType<T>
-): ComponentType<T>;
-
-function ApplicationView<T extends ComponentType>(component: T): T {
-  return view(component);
-}
+import View from './view/ApplicationView';
 
 // Export our public API.
 export default ApplicationController;
@@ -28,7 +17,7 @@ export {
   ControlledComponent,
   useController,
   // View
-  ApplicationView,
+  View as ApplicationView,
   // observer.
   raw,
   observable,

src/store/Store.ts

@@ -0,0 +1,10 @@
+import { observable } from '@nx-js/observer-util';
+
+/**
+ * Creates a store. `react-easy-state` supports detecting whether this is called in a component and
+ * wrapping with `useMemo` if so, but we don't (and shouldn't) use that, so it's unsupported here.
+ * Can wrap in user-land.
+ */
+export function store<S extends object>(obj: S | (() => S)): S {
+  return observable(typeof obj === 'function' ? obj() : obj);
+}

src/utils/addProxyInstanceOf.ts

@@ -7,9 +7,9 @@ import { raw } from '@nx-js/observer-util';
 //
 // Remove this once https://github.com/nx-js/observer-util/issues/48
 // is fixed.
-export default function (constructor) {
+export default function (constructor: { new(): unknown }) {
   Object.defineProperty(constructor, Symbol.hasInstance, {
-    value: function (instance) {
+    value: function (instance: unknown) {
       if (!instance) return false;
       let rawInstance = raw(instance);
       let rawInstanceProto = raw(Object.getPrototypeOf(rawInstance));

src/utils/randomId.ts

@@ -121,3 +121,20 @@ export const randomId = function () {
   }
   return convertBase(hex, 16, 10);
 };
+
+// TODO: replace with crypto + bigint approach from aha-app once I ensure `rand() * 2 ** randomBits`
+// is working as intended. Seems like we might want `Math.floor(rand() % (2 ** randomBits))` since
+// rand() is now generating 0-(2**32-1), not 0-1.
+
+// const rand = () => crypto.getRandomValues(new Uint32Array(1))[0];
+
+// // Generate a random ID that can be used in the database.
+// //
+// export const randomId = function () {
+//   const randomBits = 22;
+//   const time = new Date();
+//   const now = Math.floor((time.getTime() / 1000) * 1024);
+//   const entropy = Math.floor(rand() * 2 ** randomBits);
+
+//   return ((BigInt(now) << BigInt(randomBits)) | BigInt(entropy)).toString(10);
+// };

src/utils/shallowEqual.ts

@@ -0,0 +1,21 @@
+export function shallowEqual(a: unknown, b: unknown): boolean {
+  if (a === b) return true;
+  if (
+    a != null &&
+    b != null &&
+    typeof a === 'object' &&
+    typeof b === 'object'
+  ) {
+    const aKeys = Object.keys(a);
+    const bKeys = Object.keys(b);
+    return (
+      aKeys.length === bKeys.length &&
+      aKeys.every(
+        key =>
+          (a as Record<string | number, unknown>)[key] ===
+          (b as Record<string | number, unknown>)[key]
+      )
+    );
+  }
+  return false;
+}