Merge branch 'main' into fix/implementer-tools-tree-description-crash

Author: denniskigen

package.json

@@ -20,19 +20,18 @@
     "start": "openmrs develop",
     "verify": "turbo run lint test typescript",
     "postinstall": "husky install",
-    "test": "cross-env TZ=UTC jest --config jest.config.json --verbose false --passWithNoTests --color",
-    "test-watch": "cross-env TZ=UTC jest --watch --config jest.config.json --color",
+    "test": "turbo run test",
     "test-e2e": "playwright test",
-    "coverage": "yarn test --coverage"
+    "coverage": "turbo run test --coverage"
   },
   "devDependencies": {
     "@playwright/test": "^1.50.1",
     "@swc/core": "^1.11.29",
     "@swc/jest": "^0.2.38",
-    "@testing-library/dom": "^9.3.4",
-    "@testing-library/jest-dom": "^6.1.5",
-    "@testing-library/react": "^14.1.2",
-    "@testing-library/user-event": "^14.5.2",
+    "@testing-library/dom": "^10.4.1",
+    "@testing-library/jest-dom": "^6.8.0",
+    "@testing-library/react": "^16.3.0",
+    "@testing-library/user-event": "^14.6.1",
     "@types/jest": "^29.5.12",
     "@types/react": "^18.3.21",
     "@types/react-dom": "^18.3.0",

packages/apps/esm-implementer-tools-app/jest.config.js

@@ -11,6 +11,7 @@ module.exports = {
     '^lodash-es$': 'lodash',
     '^lodash-es/(.*)$': 'lodash/$1',
     '\\.(s?css)$': 'identity-obj-proxy',
+    '@openmrs/esm-framework/src/internal': '@openmrs/esm-framework/mock',
     '@openmrs/esm-framework': '@openmrs/esm-framework/mock',
     dexie: require.resolve('dexie'),
   },

packages/framework/esm-config/src/module-config/module-config.test.ts

@@ -1031,6 +1031,13 @@ describe('implementer tools config', () => {
         _type: Type.Array,
         _value: [],
       },
+      expression: {
+        _default: undefined,
+        _description: expect.any(String),
+        _source: 'default',
+        _type: Type.String,
+        _value: undefined,
+      },
     },
     'Translation overrides': {
       _default: {},
@@ -1261,7 +1268,7 @@ describe('extension config', () => {
     expect(result).toStrictEqual({
       bar: 'qux',
       baz: 'bazzy',
-      'Display conditions': { privileges: [] },
+      'Display conditions': { expression: undefined, privileges: [] },
       'Translation overrides': {},
     });
     expect(console.error).not.toHaveBeenCalled();
@@ -1284,7 +1291,7 @@ describe('extension config', () => {
     expect(result).toStrictEqual({
       bar: 'qux',
       baz: 'quiz',
-      'Display conditions': { privileges: [] },
+      'Display conditions': { expression: undefined, privileges: [] },
       'Translation overrides': {},
     });
     expect(console.error).not.toHaveBeenCalled();
@@ -1317,10 +1324,9 @@ describe('extension config', () => {
     const result = getExtensionConfig('barSlot', 'fooExt').getState().config;
     expect(result).toStrictEqual({
       qux: 'quxolotl',
-      'Display conditions': { privileges: [] },
+      'Display conditions': { expression: undefined, privileges: [] },
       'Translation overrides': {},
     });
-    expect(console.error).not.toHaveBeenCalled();
   });
 
   it("uses the 'configure' config if one is present, with extension config schema", () => {
@@ -1343,7 +1349,7 @@ describe('extension config', () => {
     const result = getExtensionConfig('barSlot', 'fooExt#id2').getState().config;
     expect(result).toStrictEqual({
       qux: 'quxotic',
-      'Display conditions': { privileges: [] },
+      'Display conditions': { expression: undefined, privileges: [] },
       'Translation overrides': {},
     });
   });

packages/framework/esm-config/src/module-config/module-config.ts

@@ -1,19 +1,19 @@
 /** @module @category Config */
-import { clone, reduce, mergeDeepRight, equals, omit } from 'ramda';
+import { clone, equals, reduce, mergeDeepRight, omit } from 'ramda';
 import type { Config, ConfigObject, ConfigSchema, ExtensionSlotConfig } from '../types';
 import { Type } from '../types';
 import { isArray, isBoolean, isUuid, isNumber, isObject, isString } from '../validators/type-validators';
 import { validator } from '../validators/validator';
 import { type ConfigExtensionStore, type ConfigInternalStore, type ConfigStore } from './state';
 import {
-  configInternalStore,
   configExtensionStore,
+  configInternalStore,
   getConfigStore,
   getExtensionConfig,
+  getExtensionSlotsConfigStore,
   getExtensionsConfigStore,
   implementerToolsConfigStore,
   temporaryConfigStore,
-  getExtensionSlotsConfigStore,
 } from './state';
 import { type TemporaryConfigStore } from '..';
 
@@ -37,42 +37,37 @@ import { type TemporaryConfigStore } from '..';
  * store values at the end. `computeExtensionConfigs` calls `getGlobalStore`,
  * which creates stores.
  */
-computeModuleConfig(configInternalStore.getState(), temporaryConfigStore.getState());
-configInternalStore.subscribe((configState) => computeModuleConfig(configState, temporaryConfigStore.getState()));
-temporaryConfigStore.subscribe((tempConfigState) =>
-  computeModuleConfig(configInternalStore.getState(), tempConfigState),
-);
-
-computeImplementerToolsConfig(configInternalStore.getState(), temporaryConfigStore.getState());
-configInternalStore.subscribe((configState) =>
-  computeImplementerToolsConfig(configState, temporaryConfigStore.getState()),
-);
-temporaryConfigStore.subscribe((tempConfigState) =>
-  computeImplementerToolsConfig(configInternalStore.getState(), tempConfigState),
-);
-
-computeExtensionSlotConfigs(configInternalStore.getState(), temporaryConfigStore.getState());
-configInternalStore.subscribe((configState) =>
-  computeExtensionSlotConfigs(configState, temporaryConfigStore.getState()),
-);
-temporaryConfigStore.subscribe((tempConfigState) =>
-  computeExtensionSlotConfigs(configInternalStore.getState(), tempConfigState),
-);
-
-computeExtensionConfigs(
-  configInternalStore.getState(),
-  configExtensionStore.getState(),
-  temporaryConfigStore.getState(),
-);
-configInternalStore.subscribe((configState) => {
-  computeExtensionConfigs(configState, configExtensionStore.getState(), temporaryConfigStore.getState());
-});
-configExtensionStore.subscribe((extensionState) => {
-  computeExtensionConfigs(configInternalStore.getState(), extensionState, temporaryConfigStore.getState());
-});
-temporaryConfigStore.subscribe((tempConfigState) => {
-  computeExtensionConfigs(configInternalStore.getState(), configExtensionStore.getState(), tempConfigState);
-});
+// Store unsubscribe functions to allow cleanup (e.g., in tests or hot module reloading)
+const configSubscriptions: Array<() => void> = [];
+
+/**
+ * Recomputes all configuration derived stores based on current state of input stores.
+ * Called whenever any input store (configInternalStore, temporaryConfigStore, configExtensionStore) changes.
+ */
+function recomputeAllConfigs() {
+  const configState = configInternalStore.getState();
+  const tempConfigState = temporaryConfigStore.getState();
+  const extensionState = configExtensionStore.getState();
+
+  computeModuleConfig(configState, tempConfigState);
+  computeImplementerToolsConfig(configState, tempConfigState);
+  computeExtensionSlotConfigs(configState, tempConfigState);
+  computeExtensionConfigs(configState, extensionState, tempConfigState);
+}
+
+function setupConfigSubscriptions() {
+  // Initial computation
+  recomputeAllConfigs();
+
+  // Subscribe to all input stores with a single handler
+  // This ensures we only recompute once even if multiple stores change simultaneously
+  configSubscriptions.push(configInternalStore.subscribe(recomputeAllConfigs));
+  configSubscriptions.push(temporaryConfigStore.subscribe(recomputeAllConfigs));
+  configSubscriptions.push(configExtensionStore.subscribe(recomputeAllConfigs));
+}
+
+// Set up subscriptions at module load time
+setupConfigSubscriptions();
 
 function computeModuleConfig(state: ConfigInternalStore, tempState: TemporaryConfigStore) {
   for (let moduleName of Object.keys(state.schemas)) {
@@ -83,21 +78,24 @@ function computeModuleConfig(state: ConfigInternalStore, tempState: TemporaryCon
     // available, which as of this writing blocks the schema definition from occurring
     // for modules loaded based on their extensions.
     const moduleStore = getConfigStore(moduleName);
+    let newState;
     if (state.moduleLoaded[moduleName]) {
       const config = getConfigForModule(moduleName, state, tempState);
-      moduleStore.setState({
+      newState = {
         translationOverridesLoaded: true,
         loaded: true,
         config,
-      });
+      };
     } else {
       const config = getConfigForModuleImplicitSchema(moduleName, state, tempState);
-      moduleStore.setState({
+      newState = {
         translationOverridesLoaded: true,
         loaded: false,
         config,
-      });
+      };
     }
+
+    moduleStore.setState(newState);
   }
 }
 
@@ -109,14 +107,21 @@ function computeExtensionSlotConfigs(state: ConfigInternalStore, tempState: Temp
   const slotStore = getExtensionSlotsConfigStore();
   const oldState = slotStore.getState();
   const newState = { slots: { ...oldState.slots, ...newSlotStoreEntries } };
-  if (!equals(oldState, newState)) {
+
+  if (!equals(oldState.slots, newState.slots)) {
     slotStore.setState(newState);
   }
 }
 
 function computeImplementerToolsConfig(state: ConfigInternalStore, tempConfigState: TemporaryConfigStore) {
+  const oldState = implementerToolsConfigStore.getState();
   const config = getImplementerToolsConfig(state, tempConfigState);
-  implementerToolsConfigStore.setState({ config });
+  const newState = { config };
+
+  // Use deep equality on the actual config content, not the wrapper object
+  if (!equals(oldState.config, newState.config)) {
+    implementerToolsConfigStore.setState(newState);
+  }
 }
 
 function computeExtensionConfigs(
@@ -137,12 +142,19 @@ function computeExtensionConfigs(
       tempConfigState,
     );
 
-    configs[extension.slotName] = {
-      ...configs[extension.slotName],
-      [extension.extensionId]: { config, loaded: true },
-    };
+    if (!configs[extension.slotName]) {
+      configs[extension.slotName] = {};
+    }
+    configs[extension.slotName][extension.extensionId] = { config, loaded: true };
+  }
+  const extensionsConfigStore = getExtensionsConfigStore();
+  const oldState = extensionsConfigStore.getState();
+  const newState = { configs };
+
+  // Use deep equality to only update if configs actually changed
+  if (!equals(oldState.configs, newState.configs)) {
+    extensionsConfigStore.setState(newState);
   }
-  getExtensionsConfigStore().setState({ configs });
 }
 
 /*
@@ -862,6 +874,20 @@ export function clearConfigErrors(keyPath?: string) {
   }
 }
 
+/**
+ * Cleans up all config store subscriptions and re-establishes them. This is primarily
+ * useful for testing, where subscriptions set up at module load time need to be cleared
+ * between tests to prevent infinite update loops. After clearing, subscriptions are
+ * re-established so the config system continues to work normally.
+ *
+ * @internal
+ */
+export function resetConfigSystem() {
+  configSubscriptions.forEach((unsubscribe) => unsubscribe());
+  configSubscriptions.length = 0;
+  setupConfigSubscriptions();
+}
+
 /**
  * Copied over from esm-extensions. It rightly belongs to that module, but esm-config
  * cannot depend on esm-extensions.
@@ -905,6 +931,11 @@ const implicitConfigSchema: ConfigSchema = {
       _type: Type.Array,
       _default: [],
     },
+    expression: {
+      _description: 'The expression that determines whether the extension is displayed',
+      _type: Type.String,
+      _default: undefined,
+    },
   },
   ...translationOverridesSchema,
 };

packages/framework/esm-config/src/module-config/state.ts

@@ -111,7 +111,7 @@ function initializeConfigStore() {
 
 /** @internal */
 export function getConfigStore(moduleName: string) {
-  // We use a store for each module's config, named `config-${moduleName}`
+  // We use a store for each module's config, named `config-module-${moduleName}`
   return getGlobalStore<ConfigStore>(`config-module-${moduleName}`, initializeConfigStore());
 }
 

packages/framework/esm-config/src/setup-tests.js

@@ -1,10 +1,12 @@
 /* eslint-disable no-undef */
+import { vi } from 'vitest';
+
 global.window.System = {
-  import: jest.fn().mockRejectedValue(new Error('config.json not available in import map')),
-  resolve: jest.fn().mockImplementation(() => {
+  import: vi.fn().mockRejectedValue(new Error('config.json not available in import map')),
+  resolve: vi.fn().mockImplementation(() => {
     throw new Error('config.json not available in import map');
   }),
-  register: jest.fn(),
+  register: vi.fn(),
 };
 
 global.window.openmrsBase = '/openmrs';

packages/framework/esm-expression-evaluator/src/evaluator.ts

@@ -242,6 +242,13 @@ export function evaluateAsType<T>(
     throw `Unknown expression type ${expression}. Expressions must either be a string or pre-compiled string.`;
   }
 
+  if (typeof expression === 'string' && expression.trim().length === 0) {
+    throw {
+      type: 'Empty expression',
+      message: 'Expression cannot be an empty string',
+    };
+  }
+
   if (typeof variables === 'undefined' || variables === null) {
     variables = {};
   }
@@ -286,6 +293,13 @@ export async function evaluateAsTypeAsync<T>(
     );
   }
 
+  if (typeof expression === 'string' && expression.trim().length === 0) {
+    throw {
+      type: 'Empty expression',
+      message: 'Expression cannot be an empty string',
+    };
+  }
+
   if (typeof variables === 'undefined' || variables === null) {
     variables = {};
   }