(fix) O3-2287: Config system reports unknown config keys for modules …

…that haven't loaded yet (#898)

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

@@ -15,6 +15,9 @@ import {
 import { Type } from '../types';
 import { getExtensionSlotsConfigStore } from '..';
 
+// Names from Wikipedia's "Metasyntactic variable" page:
+// foo, bar, baz, qux, quux, corge, grault, garply, waldo, fred, plugh, xyzzy, thud
+
 const mockConfigInternalStore = configInternalStore as MockedStore<ConfigInternalStore>;
 const mockTemporaryConfigStore = temporaryConfigStore as MockedStore<object>;
 const mockImplementerToolsConfigStore = implementerToolsConfigStore as MockedStore<ImplementerToolsConfigStore>;
@@ -734,6 +737,26 @@ describe('processConfig', () => {
 describe('implementer tools config', () => {
   afterEach(resetAll);
 
+  const implicitConfigImplementerToolsResult = {
+    'Display conditions': {
+      privileges: {
+        _default: [],
+        _description: expect.any(String),
+        _source: 'default',
+        _type: Type.Array,
+        _value: [],
+      },
+    },
+    'Translation overrides': {
+      _default: {},
+      _description: expect.any(String),
+      _source: 'default',
+      _type: Type.Object,
+      _validators: [expect.any(Function)],
+      _value: {},
+    },
+  };
+
   it('returns all config schemas, with values and sources interpolated', async () => {
     Config.defineConfigSchema('foo-module', {
       foo: { _default: 'qux', _description: 'All the foo', _validators: [] },
@@ -751,43 +774,11 @@ describe('implementer tools config', () => {
           _description: 'All the foo',
           _validators: [],
         },
-        'Display conditions': {
-          privileges: {
-            _default: [],
-            _description: 'The privilege(s) the user must have to use this extension',
-            _source: 'default',
-            _type: Type.Array,
-            _value: [],
-          },
-        },
-        'Translation overrides': {
-          _default: {},
-          _description:
-            'Per-language overrides for frontend translations should be keyed by language code and each language dictionary contains the translation key and the display value',
-          _source: 'default',
-          _type: Type.Object,
-          _value: {},
-        },
+        ...implicitConfigImplementerToolsResult,
       },
       'bar-module': {
         bar: { _value: 'baz', _source: 'my config source', _default: 'quinn' },
-        'Display conditions': {
-          privileges: {
-            _default: [],
-            _description: 'The privilege(s) the user must have to use this extension',
-            _source: 'default',
-            _type: Type.Array,
-            _value: [],
-          },
-        },
-        'Translation overrides': {
-          _default: {},
-          _description:
-            'Per-language overrides for frontend translations should be keyed by language code and each language dictionary contains the translation key and the display value',
-          _source: 'default',
-          _type: Type.Object,
-          _value: {},
-        },
+        ...implicitConfigImplementerToolsResult,
       },
     });
   });
@@ -932,23 +923,8 @@ describe('extension slot config', () => {
             remove: { _value: ['bar'], _source: 'provided' },
           },
         },
-        'Display conditions': {
-          privileges: {
-            _default: [],
-            _description: 'The privilege(s) the user must have to use this extension',
-            _source: 'default',
-            _type: Type.Array,
-            _value: [],
-          },
-        },
-        'Translation overrides': {
-          _default: {},
-          _description:
-            'Per-language overrides for frontend translations should be keyed by language code and each language dictionary contains the translation key and the display value',
-          _source: 'default',
-          _type: Type.Object,
-          _value: {},
-        },
+        'Display conditions': expect.any(Object),
+        'Translation overrides': expect.any(Object),
       },
     });
   });
@@ -1070,6 +1046,7 @@ describe('extension config', () => {
   });
 
   it('does not accept module config parameters for extension if extension config schema is defined', () => {
+    updateConfigExtensionStore();
     Config.defineExtensionConfigSchema('fooExt', {
       qux: { _default: 'quxxy' },
     });
@@ -1097,6 +1074,35 @@ describe('extension config', () => {
   });
 });
 
+describe('translation overrides', () => {
+  it('allows obtaining translation overrides before schema is registered', async () => {
+    console.error = jest.fn();
+    Config.provide({
+      'corge-module': {
+        'Translation overrides': {
+          en: {
+            'foo.bar': 'baz',
+          },
+        },
+        corges: true,
+      },
+    });
+    Config.registerModuleWithConfigSystem('corge-module');
+    const translationOverrides = await Config.getTranslationOverrides('corge-module');
+    expect(translationOverrides).toStrictEqual({
+      en: {
+        'foo.bar': 'baz',
+      },
+    });
+    Config.defineConfigSchema('corge-module', {
+      corges: { _default: false, _type: Type.Boolean },
+    });
+    const config = Config.getConfig('corge-module');
+    expect(config).resolves.toStrictEqual({ corges: true });
+    expect(console.error).not.toHaveBeenCalled();
+  });
+});
+
 function updateConfigExtensionStore(extensionId = 'fooExt') {
   configExtensionStore.setState({
     mountedExtensions: [

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

@@ -3,6 +3,7 @@ import { clone, reduce, mergeDeepRight, equals, omit } from 'ramda';
 import type { Config, ConfigObject, ConfigSchema, ExtensionSlotConfig, ExtensionSlotConfigObject } 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, ConfigInternalStore, ConfigStore } from './state';
 import {
   configInternalStore,
@@ -14,7 +15,7 @@ import {
   getExtensionSlotsConfigStore,
 } from './state';
 import type {} from '@openmrs/esm-globals';
-import type { TemporaryConfigStore } from '..';
+import { type TemporaryConfigStore } from '..';
 
 /**
  * Store setup
@@ -75,9 +76,28 @@ temporaryConfigStore.subscribe((tempConfigState) => {
 
 function computeModuleConfig(state: ConfigInternalStore, tempState: TemporaryConfigStore) {
   for (let moduleName of Object.keys(state.schemas)) {
-    const config = getConfigForModule(moduleName, state, tempState);
+    // At this point the schema could be either just the implicit schema or the actually
+    // defined schema. We run with just the implicit schema because we want to populate
+    // the config store with the translation overrides as soon as possible. In fact, the
+    // translation system will throw for Suspense until the translation overrides are
+    // available, which as of this writing blocks the schema definition from occurring
+    // for modules loaded based on their extensions.
     const moduleStore = getConfigStore(moduleName);
-    moduleStore.setState({ loaded: true, config });
+    if (state.moduleLoaded[moduleName]) {
+      const config = getConfigForModule(moduleName, state, tempState);
+      moduleStore.setState({
+        translationOverridesLoaded: true,
+        loaded: true,
+        config,
+      });
+    } else {
+      const config = getConfigForModuleImplicitSchema(moduleName, state, tempState);
+      moduleStore.setState({
+        translationOverridesLoaded: true,
+        loaded: false,
+        config,
+      });
+    }
   }
 }
 
@@ -105,6 +125,8 @@ function computeExtensionConfigs(
   tempConfigState: TemporaryConfigStore,
 ) {
   const configs = {};
+  // We assume that the module schema has already been defined, since the extension
+  // it contains is mounted.
   for (let extension of extensionState.mountedExtensions) {
     const config = computeExtensionConfig(
       extension.slotModuleName,
@@ -146,6 +168,24 @@ export function defineConfigSchema(moduleName: string, schema: ConfigSchema) {
   const state = configInternalStore.getState();
   configInternalStore.setState({
     schemas: { ...state.schemas, [moduleName]: enhancedSchema },
+    moduleLoaded: { ...state.moduleLoaded, [moduleName]: true },
+  });
+}
+
+/**
+ * This alerts the configuration system that a module exists. This allows config to be
+ * processed, while still allowing the extension system to know whether the module has
+ * actually had its front bundle executed yet.
+ *
+ * This should only be used in esm-app-shell.
+ *
+ * @internal
+ * @param moduleName
+ */
+export function registerModuleWithConfigSystem(moduleName: string) {
+  const state = configInternalStore.getState();
+  configInternalStore.setState({
+    schemas: { ...state.schemas, [moduleName]: implicitConfigSchema },
   });
 }
 
@@ -214,13 +254,13 @@ export function getConfig<T = Record<string, any>>(moduleName: string): Promise<
 }
 
 /** @internal */
-export function getConfigInternal(moduleName: string): Promise<Config> {
-  return new Promise<Config>((resolve) => {
+export function getTranslationOverrides(moduleName: string): Promise<Object> {
+  return new Promise<Object>((resolve) => {
     const store = getConfigStore(moduleName);
     function update(state: ConfigStore) {
-      if (state.loaded && state.config) {
-        const config = state.config;
-        resolve(config);
+      if (state.translationOverridesLoaded && state.config) {
+        const translationOverrides = state.config['Translation overrides'] ?? {};
+        resolve(translationOverrides);
         unsubscribe && unsubscribe();
       }
     }
@@ -279,7 +319,6 @@ function computeExtensionConfig(
   const configOverride = slotModuleConfig?.extensionSlots?.[slotName]?.configure?.[extensionId] ?? {};
   const extensionConfig = mergeConfigsFor(nameOfSchemaSource, providedConfigs);
   const combinedConfig = mergeConfigs([extensionConfig, configOverride]);
-  // TODO: validate that a schema exists for the module
   const schema = extensionConfigSchema ?? configState.schemas[extensionModuleName];
   validateStructure(schema, combinedConfig, nameOfSchemaSource);
   const config = setDefaults(schema, combinedConfig);
@@ -477,10 +516,6 @@ function getConfigForModule(
   configState: ConfigInternalStore,
   tempConfigState: TemporaryConfigStore,
 ): ConfigObject {
-  if (!configState.schemas.hasOwnProperty(moduleName)) {
-    throw Error('No config schema has been defined for ' + moduleName);
-  }
-
   const schema = configState.schemas[moduleName];
   const inputConfig = mergeConfigsFor(moduleName, getProvidedConfigs(configState, tempConfigState));
   validateStructure(schema, inputConfig, moduleName);
@@ -490,6 +525,18 @@ function getConfigForModule(
   return config;
 }
 
+function getConfigForModuleImplicitSchema(
+  moduleName: string,
+  configState: ConfigInternalStore,
+  tempConfigState: TemporaryConfigStore,
+): ConfigObject {
+  const inputConfig = mergeConfigsFor(moduleName, getProvidedConfigs(configState, tempConfigState));
+  const config = setDefaults(implicitConfigSchema, inputConfig);
+  runAllValidatorsInConfigTree(implicitConfigSchema, config, moduleName);
+  delete config.extensionSlots;
+  return config;
+}
+
 function mergeConfigsFor(moduleName: string, allConfigs: Array<Config>): ConfigObject {
   const allConfigsForModule = allConfigs.map(({ [moduleName]: c }) => c).filter((c) => !!c);
 
@@ -751,5 +798,16 @@ const implicitConfigSchema: ConfigSchema = {
       'Per-language overrides for frontend translations should be keyed by language code and each language dictionary contains the translation key and the display value',
     _type: Type.Object,
     _default: {},
+    _validators: [
+      validator(
+        (o) => Object.keys(o).every((k) => /^[a-z]{2,3}(-[A-Z]{2,3})?$/.test(k)),
+        (o) => {
+          const badKeys = Object.keys(o).filter((k) => !/^[a-z]{2,3}(-[A-Z]{2,3})?$/.test(k));
+          return `The 'Translation overrides' object should have language codes for keys. Language codes must be in the form of a two-to-three letter language code optionally followed by a hyphen and a two-to-three letter country code. The following keys do not conform: ${badKeys.join(
+            ', ',
+          )}.`;
+        },
+      ),
+    ],
   },
 };

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

@@ -13,12 +13,18 @@ export interface ConfigInternalStore {
   providedConfigs: Array<ProvidedConfig>;
   /** An object with module names for keys and schemas for values */
   schemas: Record<string, ConfigSchema>;
-  /** Whether to use dev defaults or not */
+  /**
+   * Before modules are loaded, they get implicit schemas added to `schemas`. Therefore
+   * we need to track separately whether they have actually been loaded (that is,
+   * whether the schema has actually been defined).
+   */
+  moduleLoaded: Record<string, boolean>;
 }
 
 const configInternalStoreInitialValue = {
   providedConfigs: [],
   schemas: {},
+  moduleLoaded: {},
 };
 
 /**
@@ -92,12 +98,14 @@ export const configExtensionStore = createGlobalStore<ConfigExtensionStore>('con
 export interface ConfigStore {
   config: ConfigObject | null;
   loaded: boolean;
+  translationOverridesLoaded: boolean;
 }
 
 function initializeConfigStore() {
   return {
     config: null,
     loaded: false,
+    translationOverridesLoaded: false,
   };
 }
 

packages/framework/esm-react-utils/src/useConfig.ts

@@ -137,9 +137,9 @@ interface UseConfigOptions {
  * @param options Additional options that can be passed to useConfig()
  */
 export function useConfig<T = Record<string, any>>(options?: UseConfigOptions) {
-  // This hook uses the config of the MF defining the component.
-  // If the component is used in an extension slot then the slot
-  // may override (part of) its configuration.
+  // This hook gets the appropriate configuration depending on whether the caller is a module
+  // or an extension, which is determined from the ComponentContext. It will throw for suspense
+  // if the configuration is not yet loaded.
   const { moduleName: contextModuleName, extension } = useContext(ComponentContext);
   const moduleName = options?.externalModuleName ?? contextModuleName;
 

packages/shell/esm-app-shell/src/apps.ts

@@ -5,9 +5,10 @@ import type {
   RouteDefinition,
   ExtensionRegistration,
 } from '@openmrs/esm-framework';
-import { attach, registerExtension, defineConfigSchema, importDynamic } from '@openmrs/esm-framework';
+import { attach, registerExtension, importDynamic } from '@openmrs/esm-framework';
 import { type ActivityFn, type LifeCycles, pathToActiveWhen, registerApplication } from 'single-spa';
 import { emptyLifecycle, routeRegex } from './helpers';
+import { registerModuleWithConfigSystem } from '@openmrs/esm-framework/src/internal';
 
 const pages: Array<RegisteredPageDefinition> = [];
 
@@ -127,7 +128,7 @@ function getLoader(appName: string, component: string): () => Promise<LifeCycles
  */
 export function registerApp(appName: string, routes: OpenmrsAppRoutes) {
   if (appName && routes && typeof routes === 'object') {
-    defineConfigSchema(appName, {});
+    registerModuleWithConfigSystem(appName);
 
     const availableExtensions: Array<ExtensionDefinition> = routes.extensions ?? [];