diff --git a/package-lock.json b/package-lock.json index 3ceef165..cb245d16 100644 --- a/package-lock.json +++ b/package-lock.json @@ -23,7 +23,6 @@ "@tanstack/react-query-devtools": "^5.99.0", "@types/eslint__js": "^8.42.3", "@types/gradient-string": "^1.1.6", - "@types/lodash.keyby": "^4.6.9", "autoprefixer": "^10.4.20", "axios": "^1.7.9", "axios-cache-interceptor": "^1.6.0", @@ -57,11 +56,7 @@ "jwt-decode": "^3.1.2", "localforage": "^1.10.0", "localforage-memoryStorageDriver": "^0.9.2", - "lodash.camelcase": "^4.3.0", - "lodash.keyby": "^4.6.0", - "lodash.memoize": "^4.1.2", - "lodash.merge": "^4.6.2", - "lodash.snakecase": "^4.1.1", + "lodash": "^4.18.1", "mini-css-extract-plugin": "1.6.2", "parse5": "7.3.0", "postcss": "^8.4.47", @@ -108,8 +103,7 @@ "@tsconfig/node24": "^24.0.4", "@types/compression": "^1.7.5", "@types/jest": "^29.5.14", - "@types/lodash.camelcase": "^4.3.9", - "@types/lodash.merge": "^4.6.9", + "@types/lodash": "^4.17.24", "@types/node": "^24.12.0", "@types/react": "^18.3.20", "@types/react-dom": "^18.3.6", @@ -2124,6 +2118,13 @@ "lodash-es": "4.17.23" } }, + "node_modules/@chevrotain/cst-dts-gen/node_modules/lodash-es": { + "version": "4.17.23", + "resolved": "https://registry.npmjs.org/lodash-es/-/lodash-es-4.17.23.tgz", + "integrity": "sha512-kVI48u3PZr38HdYz98UmfPnXl2DXrpdctLrFLCd3kOx1xUkOmpFPx7gCWWM5MPkL/fD8zb+Ph0QzjGFs4+hHWg==", + "license": "MIT", + "peer": true + }, "node_modules/@chevrotain/gast": { "version": "11.2.0", "resolved": "https://registry.npmjs.org/@chevrotain/gast/-/gast-11.2.0.tgz", @@ -2135,6 +2136,13 @@ "lodash-es": "4.17.23" } }, + "node_modules/@chevrotain/gast/node_modules/lodash-es": { + "version": "4.17.23", + "resolved": "https://registry.npmjs.org/lodash-es/-/lodash-es-4.17.23.tgz", + "integrity": "sha512-kVI48u3PZr38HdYz98UmfPnXl2DXrpdctLrFLCd3kOx1xUkOmpFPx7gCWWM5MPkL/fD8zb+Ph0QzjGFs4+hHWg==", + "license": "MIT", + "peer": true + }, "node_modules/@chevrotain/regexp-to-ast": { "version": "11.2.0", "resolved": "https://registry.npmjs.org/@chevrotain/regexp-to-ast/-/regexp-to-ast-11.2.0.tgz", @@ -2940,9 +2948,6 @@ "cpu": [ "arm" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -2959,9 +2964,6 @@ "cpu": [ "arm64" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -2978,9 +2980,6 @@ "cpu": [ "ppc64" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -2997,9 +2996,6 @@ "cpu": [ "riscv64" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -3016,9 +3012,6 @@ "cpu": [ "s390x" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -3035,9 +3028,6 @@ "cpu": [ "x64" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -3054,9 +3044,6 @@ "cpu": [ "arm64" ], - "libc": [ - "musl" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -3073,9 +3060,6 @@ "cpu": [ "x64" ], - "libc": [ - "musl" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -3092,9 +3076,6 @@ "cpu": [ "arm" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -3117,9 +3098,6 @@ "cpu": [ "arm64" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -3142,9 +3120,6 @@ "cpu": [ "ppc64" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -3167,9 +3142,6 @@ "cpu": [ "riscv64" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -3192,9 +3164,6 @@ "cpu": [ "s390x" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -3217,9 +3186,6 @@ "cpu": [ "x64" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -3242,9 +3208,6 @@ "cpu": [ "arm64" ], - "libc": [ - "musl" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -3267,9 +3230,6 @@ "cpu": [ "x64" ], - "libc": [ - "musl" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -5695,36 +5655,8 @@ "version": "4.17.24", "resolved": "https://registry.npmjs.org/@types/lodash/-/lodash-4.17.24.tgz", "integrity": "sha512-gIW7lQLZbue7lRSWEFql49QJJWThrTFFeIMJdp3eH4tKoxm1OvEPg02rm4wCCSHS0cL3/Fizimb35b7k8atwsQ==", - "license": "MIT" - }, - "node_modules/@types/lodash.camelcase": { - "version": "4.3.9", - "resolved": "https://registry.npmjs.org/@types/lodash.camelcase/-/lodash.camelcase-4.3.9.tgz", - "integrity": "sha512-ys9/hGBfsKxzmFI8hckII40V0ASQ83UM2pxfQRghHAwekhH4/jWtjz/3/9YDy7ZpUd/H0k2STSqmPR28dnj7Zg==", - "dev": true, - "license": "MIT", - "dependencies": { - "@types/lodash": "*" - } - }, - "node_modules/@types/lodash.keyby": { - "version": "4.6.9", - "resolved": "https://registry.npmjs.org/@types/lodash.keyby/-/lodash.keyby-4.6.9.tgz", - "integrity": "sha512-N8xfQdZ2ADNPDL72TaLozIL4K1xFCMG1C1T9GN4dOFI+sn1cjl8d4U+POp8PRCAnNxDCMkYAZVD/rOBIWYPT5g==", - "license": "MIT", - "dependencies": { - "@types/lodash": "*" - } - }, - "node_modules/@types/lodash.merge": { - "version": "4.6.9", - "resolved": "https://registry.npmjs.org/@types/lodash.merge/-/lodash.merge-4.6.9.tgz", - "integrity": "sha512-23sHDPmzd59kUgWyKGiOMO2Qb9YtqRO/x4IhkgNUiPQ1+5MUVqi6bCZeq9nBJ17msjIMbEIO5u+XW4Kz6aGUhQ==", "dev": true, - "license": "MIT", - "dependencies": { - "@types/lodash": "*" - } + "license": "MIT" }, "node_modules/@types/markdown-it": { "version": "14.1.2", @@ -7705,6 +7637,13 @@ "lodash-es": "4.17.23" } }, + "node_modules/chevrotain/node_modules/lodash-es": { + "version": "4.17.23", + "resolved": "https://registry.npmjs.org/lodash-es/-/lodash-es-4.17.23.tgz", + "integrity": "sha512-kVI48u3PZr38HdYz98UmfPnXl2DXrpdctLrFLCd3kOx1xUkOmpFPx7gCWWM5MPkL/fD8zb+Ph0QzjGFs4+hHWg==", + "license": "MIT", + "peer": true + }, "node_modules/child_process": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/child_process/-/child_process-1.0.2.tgz", @@ -13912,31 +13851,12 @@ "integrity": "sha512-dMInicTPVE8d1e5otfwmmjlxkZoUpiVLwyeTdUsi/Caj/gfzzblBcCE5sRHV/AsjuCmxWrte2TNGSYuCeCq+0Q==", "license": "MIT" }, - "node_modules/lodash-es": { - "version": "4.17.23", - "resolved": "https://registry.npmjs.org/lodash-es/-/lodash-es-4.17.23.tgz", - "integrity": "sha512-kVI48u3PZr38HdYz98UmfPnXl2DXrpdctLrFLCd3kOx1xUkOmpFPx7gCWWM5MPkL/fD8zb+Ph0QzjGFs4+hHWg==", - "license": "MIT", - "peer": true - }, - "node_modules/lodash.camelcase": { - "version": "4.3.0", - "resolved": "https://registry.npmjs.org/lodash.camelcase/-/lodash.camelcase-4.3.0.tgz", - "integrity": "sha512-TwuEnCnxbc3rAvhf/LbG7tJUDzhqXyFnv3dtzLOPgCG/hODL7WFnsbwktkD7yUV0RrreP/l1PALq/YSg6VvjlA==", - "license": "MIT" - }, "node_modules/lodash.debounce": { "version": "4.0.8", "resolved": "https://registry.npmjs.org/lodash.debounce/-/lodash.debounce-4.0.8.tgz", "integrity": "sha512-FT1yDzDYEoYWhnSGnpE/4Kj1fLZkDFyqRb7fNt6FdYOSxlUWAtp42Eh6Wb0rGIv/m9Bgo7x4GhQbm5Ys4SG5ow==", "license": "MIT" }, - "node_modules/lodash.keyby": { - "version": "4.6.0", - "resolved": "https://registry.npmjs.org/lodash.keyby/-/lodash.keyby-4.6.0.tgz", - "integrity": "sha512-PRe4Cn20oJM2Sn6ljcZMeKgyhTHpzvzFmdsp9rK+6K0eJs6Tws0MqgGFpfX/o2HjcoQcBny1Eik9W7BnVTzjIQ==", - "license": "MIT" - }, "node_modules/lodash.memoize": { "version": "4.1.2", "resolved": "https://registry.npmjs.org/lodash.memoize/-/lodash.memoize-4.1.2.tgz", @@ -13949,12 +13869,6 @@ "integrity": "sha512-0KpjqXRVvrYyCsX1swR/XTK0va6VQkQM6MNo7PqW77ByjAhoARA8EfrP1N4+KlKj8YS0ZUCtRT/YUuhyYDujIQ==", "license": "MIT" }, - "node_modules/lodash.snakecase": { - "version": "4.1.1", - "resolved": "https://registry.npmjs.org/lodash.snakecase/-/lodash.snakecase-4.1.1.tgz", - "integrity": "sha512-QZ1d4xoBHYUeuouhEq3lk3Uq7ldgyFXGBhg04+oRLnIz8o9T65Eh+8YdroUwn846zchkA9yDsDl5CVVaV2nqYw==", - "license": "MIT" - }, "node_modules/lodash.uniq": { "version": "4.5.0", "resolved": "https://registry.npmjs.org/lodash.uniq/-/lodash.uniq-4.5.0.tgz", diff --git a/package.json b/package.json index f83fc7b6..49832f58 100644 --- a/package.json +++ b/package.json @@ -73,7 +73,6 @@ "@tanstack/react-query-devtools": "^5.99.0", "@types/eslint__js": "^8.42.3", "@types/gradient-string": "^1.1.6", - "@types/lodash.keyby": "^4.6.9", "autoprefixer": "^10.4.20", "axios": "^1.7.9", "axios-cache-interceptor": "^1.6.0", @@ -107,11 +106,7 @@ "jwt-decode": "^3.1.2", "localforage": "^1.10.0", "localforage-memoryStorageDriver": "^0.9.2", - "lodash.camelcase": "^4.3.0", - "lodash.keyby": "^4.6.0", - "lodash.memoize": "^4.1.2", - "lodash.merge": "^4.6.2", - "lodash.snakecase": "^4.1.1", + "lodash": "^4.18.1", "mini-css-extract-plugin": "1.6.2", "parse5": "7.3.0", "postcss": "^8.4.47", @@ -155,8 +150,7 @@ "@tsconfig/node24": "^24.0.4", "@types/compression": "^1.7.5", "@types/jest": "^29.5.14", - "@types/lodash.camelcase": "^4.3.9", - "@types/lodash.merge": "^4.6.9", + "@types/lodash": "^4.17.24", "@types/node": "^24.12.0", "@types/react": "^18.3.20", "@types/react-dom": "^18.3.6", diff --git a/runtime/config/index.test.ts b/runtime/config/index.test.ts index 6d064843..6c53ae47 100644 --- a/runtime/config/index.test.ts +++ b/runtime/config/index.test.ts @@ -1,11 +1,16 @@ -import { CONFIG_CHANGED } from '../constants'; +import { ACTIVE_ROLES_CHANGED, CONFIG_CHANGED } from '../constants'; import * as subscriptions from '../subscriptions'; import { + addActiveWidgetRole, addAppConfigs, + getActiveRouteRoles, + getActiveWidgetRoles, getAppConfig, getProvides, getSiteConfig, mergeSiteConfig, + removeActiveWidgetRole, + setActiveRouteRoles, setSiteConfig, } from './index'; @@ -63,6 +68,38 @@ describe('mergeSiteConfig', () => { expect(getSiteConfig().siteName).toBe('Updated'); expect(getSiteConfig().lmsBaseUrl).toBe('http://original.url'); }); + + it('does not mutate the previous siteConfig reference', () => { + const before = getSiteConfig(); + const beforeSnapshot = { ...before }; + + mergeSiteConfig({ siteName: 'Updated' }); + + const after = getSiteConfig(); + expect(after).not.toBe(before); + expect(before).toEqual(beforeSnapshot); + }); + + it('does not mutate previously-held nested arrays', () => { + setSiteConfig({ + ...defaultSiteConfig, + apps: [{ appId: 'existing', config: { VALUE: 'before' } }], + }); + const beforeApps = getSiteConfig().apps; + const beforeAppsLength = beforeApps!.length; + const beforeApp = beforeApps![0]; + const beforeAppConfig = { ...beforeApp.config }; + + mergeSiteConfig({ + apps: [{ appId: 'existing', config: { VALUE: 'after' } }], + }); + + // The previously-held array and app object must be unchanged. + expect(beforeApps!.length).toBe(beforeAppsLength); + expect(beforeApp.config).toEqual(beforeAppConfig); + // ...even though the live siteConfig sees the merged value. + expect(getSiteConfig().apps![0].config!.VALUE).toBe('after'); + }); }); describe('app merging (full merge, default behavior)', () => { @@ -436,3 +473,84 @@ describe('mergeSiteConfig', () => { }); }); }); + +describe('active role mutators', () => { + let publishSpy: jest.SpyInstance; + + beforeEach(() => { + /* Reset shared state. setActiveRouteRoles uses isEqual, so any non-empty + array different from the current one will reset; we follow up with []. */ + setActiveRouteRoles(['__reset__']); + setActiveRouteRoles([]); + for (const role of getActiveWidgetRoles()) { + while (getActiveWidgetRoles().includes(role)) { + removeActiveWidgetRole(role); + } + } + publishSpy = jest.spyOn(subscriptions, 'publish'); + }); + + afterEach(() => { + publishSpy.mockRestore(); + }); + + describe('setActiveRouteRoles', () => { + it('publishes ACTIVE_ROLES_CHANGED when the roles change', () => { + setActiveRouteRoles(['role-a']); + expect(publishSpy).toHaveBeenCalledWith(ACTIVE_ROLES_CHANGED); + expect(getActiveRouteRoles()).toEqual(['role-a']); + }); + + it('does not publish when called with a deeply-equal array', () => { + setActiveRouteRoles(['role-a']); + publishSpy.mockClear(); + + setActiveRouteRoles(['role-a']); + expect(publishSpy).not.toHaveBeenCalled(); + }); + + it('publishes again when the roles transition back to a different value', () => { + setActiveRouteRoles(['role-a']); + setActiveRouteRoles(['role-b']); + publishSpy.mockClear(); + + setActiveRouteRoles(['role-a']); + expect(publishSpy).toHaveBeenCalledWith(ACTIVE_ROLES_CHANGED); + }); + }); + + describe('addActiveWidgetRole / removeActiveWidgetRole', () => { + it('publishes only on the 0->1 transition when adding', () => { + addActiveWidgetRole('widget-role'); + expect(publishSpy).toHaveBeenCalledWith(ACTIVE_ROLES_CHANGED); + expect(getActiveWidgetRoles()).toContain('widget-role'); + + publishSpy.mockClear(); + addActiveWidgetRole('widget-role'); + addActiveWidgetRole('widget-role'); + expect(publishSpy).not.toHaveBeenCalled(); + }); + + it('publishes only on the 1->0 transition when removing', () => { + addActiveWidgetRole('widget-role'); + addActiveWidgetRole('widget-role'); + addActiveWidgetRole('widget-role'); + publishSpy.mockClear(); + + removeActiveWidgetRole('widget-role'); + removeActiveWidgetRole('widget-role'); + expect(publishSpy).not.toHaveBeenCalled(); + expect(getActiveWidgetRoles()).toContain('widget-role'); + + removeActiveWidgetRole('widget-role'); + expect(publishSpy).toHaveBeenCalledWith(ACTIVE_ROLES_CHANGED); + expect(getActiveWidgetRoles()).not.toContain('widget-role'); + }); + + it('does nothing when removing a role that was never added', () => { + removeActiveWidgetRole('never-added'); + expect(publishSpy).not.toHaveBeenCalled(); + expect(getActiveWidgetRoles()).not.toContain('never-added'); + }); + }); +}); diff --git a/runtime/config/index.ts b/runtime/config/index.ts index 7b880f71..c48d761d 100644 --- a/runtime/config/index.ts +++ b/runtime/config/index.ts @@ -100,8 +100,9 @@ * @module Config */ -import keyBy from 'lodash.keyby'; -import merge from 'lodash.merge'; +import isEqual from 'lodash/isEqual'; +import keyBy from 'lodash/keyBy'; +import merge from 'lodash/merge'; import { AppConfig, EnvironmentTypes, @@ -212,8 +213,10 @@ export function mergeSiteConfig( const { limitAppMergeToConfig = false } = options; const { apps: newApps, ...restOfNewConfig } = newSiteConfig; - // lodash merge the top-level (non-app) part - siteConfig = merge(siteConfig, restOfNewConfig); + /* `merge({}, ...)` deep-clones into the fresh target, so the new `siteConfig` + is a brand-new reference and the previous one is never mutated. This is what + lets React consumers detect that CONFIG_CHANGED actually changed something. */ + siteConfig = merge({}, siteConfig, restOfNewConfig); // if we don't have new apps, we're done if (!newApps?.length) { @@ -224,6 +227,7 @@ export function mergeSiteConfig( // if we're doing a full merge, merge the objects if (!limitAppMergeToConfig) { siteConfig.apps = Object.values(merge( + {}, keyBy(siteConfig.apps || [], 'appId'), keyBy(newApps, 'appId') )); @@ -240,12 +244,13 @@ export function mergeSiteConfig( // handle config-only merging const newAppsById = keyBy(newApps, 'appId'); - for (const app of siteConfig.apps) { + siteConfig.apps = siteConfig.apps.map((app) => { const newApp = newAppsById[app.appId]; - if (newApp?.config) { - app.config = merge(app.config, newApp.config); + if (!newApp?.config) { + return app; } - } + return { ...app, config: merge({}, app.config, newApp.config) }; + }); publish(CONFIG_CHANGED); } @@ -280,13 +285,16 @@ export function getAppConfig(id: string) { } export function mergeAppConfig(id: string, newAppConfig: AppConfig) { - appConfigs[id] = merge(appConfigs[id], newAppConfig); + // Non-mutating: produce a fresh entry so consumers holding a reference to + // the previous one don't observe the change underneath them. + appConfigs[id] = merge({}, appConfigs[id], newAppConfig); publish(CONFIG_CHANGED); } let activeRouteRoles: string[] = []; export function setActiveRouteRoles(roles: string[]) { + if (isEqual(activeRouteRoles, roles)) return; activeRouteRoles = roles; publish(ACTIVE_ROLES_CHANGED); } @@ -298,19 +306,20 @@ export function getActiveRouteRoles() { const activeWidgetRoles: Record = {}; export function addActiveWidgetRole(role: string) { - activeWidgetRoles[role] ??= 0; - activeWidgetRoles[role] += 1; - publish(ACTIVE_ROLES_CHANGED); + // Only publish when the role transitions from absent to present. + const wasPresent = (activeWidgetRoles[role] ?? 0) > 0; + activeWidgetRoles[role] = (activeWidgetRoles[role] ?? 0) + 1; + if (!wasPresent) publish(ACTIVE_ROLES_CHANGED); } export function removeActiveWidgetRole(role: string) { - if (activeWidgetRoles[role] !== undefined) { - activeWidgetRoles[role] -= 1; - } + if (activeWidgetRoles[role] === undefined) return; + activeWidgetRoles[role] -= 1; if (activeWidgetRoles[role] < 1) { delete activeWidgetRoles[role]; + // Only publish when the role transitions from present to absent. + publish(ACTIVE_ROLES_CHANGED); } - publish(ACTIVE_ROLES_CHANGED); } export function getActiveWidgetRoles() { diff --git a/runtime/i18n/lib.ts b/runtime/i18n/lib.ts index b8ee79fa..41f39653 100644 --- a/runtime/i18n/lib.ts +++ b/runtime/i18n/lib.ts @@ -1,4 +1,4 @@ -import merge from 'lodash.merge'; +import merge from 'lodash/merge'; import PropTypes from 'prop-types'; import { MessageFormatElement } from 'react-intl'; import Cookies from 'universal-cookie'; diff --git a/runtime/initialize.async.function.config.test.js b/runtime/initialize.async.function.config.test.js index 2f25c3fb..976213ad 100644 --- a/runtime/initialize.async.function.config.test.js +++ b/runtime/initialize.async.function.config.test.js @@ -21,12 +21,9 @@ jest.mock('site.config', () => async () => new Promise((resolve) => { }); })); -let config = null; - describe('initialize with async function js file config', () => { beforeEach(() => { jest.resetModules(); - config = getSiteConfig(); fetchAuthenticatedUser.mockReset(); ensureAuthenticatedUser.mockReset(); hydrateAuthenticatedUser.mockReset(); @@ -38,6 +35,6 @@ describe('initialize with async function js file config', () => { const messages = { i_am: 'a message' }; await initialize({ messages }); - expect(config.JS_FILE_VAR).toEqual('JS_FILE_VAR_VALUE_ASYNC_FUNCTION'); + expect(getSiteConfig().JS_FILE_VAR).toEqual('JS_FILE_VAR_VALUE_ASYNC_FUNCTION'); }); }); diff --git a/runtime/initialize.const.config.test.js b/runtime/initialize.const.config.test.js index dd6d8cfb..9d001639 100644 --- a/runtime/initialize.const.config.test.js +++ b/runtime/initialize.const.config.test.js @@ -19,12 +19,9 @@ jest.mock('site.config', () => ({ JS_FILE_VAR: 'JS_FILE_VAR_VALUE_CONSTANT', })); -let config = null; - describe('initialize with constant js file config', () => { beforeEach(() => { jest.resetModules(); - config = getSiteConfig(); fetchAuthenticatedUser.mockReset(); ensureAuthenticatedUser.mockReset(); hydrateAuthenticatedUser.mockReset(); @@ -36,6 +33,6 @@ describe('initialize with constant js file config', () => { const messages = { i_am: 'a message' }; await initialize({ messages }); - expect(config.JS_FILE_VAR).toEqual('JS_FILE_VAR_VALUE_CONSTANT'); + expect(getSiteConfig().JS_FILE_VAR).toEqual('JS_FILE_VAR_VALUE_CONSTANT'); }); }); diff --git a/runtime/initialize.function.config.test.js b/runtime/initialize.function.config.test.js index 1e1a96f9..2b394035 100644 --- a/runtime/initialize.function.config.test.js +++ b/runtime/initialize.function.config.test.js @@ -19,12 +19,9 @@ jest.mock('site.config', () => () => ({ JS_FILE_VAR: 'JS_FILE_VAR_VALUE_FUNCTION', })); -let config = null; - describe('initialize with function js file config', () => { beforeEach(() => { jest.resetModules(); - config = getSiteConfig(); fetchAuthenticatedUser.mockReset(); ensureAuthenticatedUser.mockReset(); hydrateAuthenticatedUser.mockReset(); @@ -36,6 +33,6 @@ describe('initialize with function js file config', () => { const messages = { i_am: 'a message' }; await initialize({ messages }); - expect(config.JS_FILE_VAR).toEqual('JS_FILE_VAR_VALUE_FUNCTION'); + expect(getSiteConfig().JS_FILE_VAR).toEqual('JS_FILE_VAR_VALUE_FUNCTION'); }); }); diff --git a/runtime/react/CombinedAppProvider.tsx b/runtime/react/CombinedAppProvider.tsx index a4c9e690..654670f8 100644 --- a/runtime/react/CombinedAppProvider.tsx +++ b/runtime/react/CombinedAppProvider.tsx @@ -1,22 +1,7 @@ -import { ReactNode } from 'react'; +import { ReactNode, useMemo } from 'react'; import { App, AppProvider } from '../../types'; import { getSiteConfig } from '../config'; -const combineProviders = (providers: AppProvider[]): AppProvider => { - return providers.reduce( - (AccumulatedProviders, CurrentProvider) => { - // eslint-disable-next-line react/prop-types - const CombinedProvider: AppProvider = ({ children }) => ( - - {children} - - ); - return CombinedProvider; - }, - ({ children }) => <>{children}, - ); -}; - interface CombinedAppProviderProps { children: ReactNode, } @@ -24,23 +9,20 @@ interface CombinedAppProviderProps { export default function CombinedAppProvider({ children }: CombinedAppProviderProps) { const { apps } = getSiteConfig(); - let providers: AppProvider[] = []; - - if (apps) { - apps.forEach( - (app: App) => { + const providers = useMemo(() => { + const list: AppProvider[] = []; + if (apps) { + apps.forEach((app: App) => { if (Array.isArray(app.providers)) { - providers = providers.concat(app.providers); + list.push(...app.providers); } - } - ); - } - - const CombinedProviders = combineProviders(providers); + }); + } + return list; + }, [apps]); - return ( - - {children} - + return providers.reduceRight( + (acc, Provider) => {acc}, + children, ); }; diff --git a/runtime/react/hooks/useActiveRoles.ts b/runtime/react/hooks/useActiveRoles.ts index bef7327c..1cd81107 100644 --- a/runtime/react/hooks/useActiveRoles.ts +++ b/runtime/react/hooks/useActiveRoles.ts @@ -1,13 +1,16 @@ -import { useState } from 'react'; +import { useCallback, useState } from 'react'; import { getActiveRoles } from '../../config'; import { ACTIVE_ROLES_CHANGED } from '../../constants'; import useSiteEvent from './useSiteEvent'; const useActiveRoles = () => { const [roles, setRoles] = useState(getActiveRoles()); - useSiteEvent(ACTIVE_ROLES_CHANGED, () => { + /* Stabilize the callback so useSiteEvent doesn't unsubscribe + resubscribe on every render. + Every Slot in the tree subscribes via this hook, so the churn would be tree-wide. */ + const onChange = useCallback(() => { setRoles(getActiveRoles()); - }); + }, []); + useSiteEvent(ACTIVE_ROLES_CHANGED, onChange); return roles; }; diff --git a/runtime/react/hooks/useActiveRouteRoleWatcher.ts b/runtime/react/hooks/useActiveRouteRoleWatcher.ts index 44359803..8c9a8d3f 100644 --- a/runtime/react/hooks/useActiveRouteRoleWatcher.ts +++ b/runtime/react/hooks/useActiveRouteRoleWatcher.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect } from 'react'; +import { useEffect } from 'react'; import { useMatches } from 'react-router'; import { setActiveRouteRoles } from '../../config'; import { isRoleRouteObject } from '../../routing'; @@ -6,12 +6,8 @@ import { isRoleRouteObject } from '../../routing'; const useActiveRouteRoleWatcher = () => { const matches = useMatches(); - // We create this callback so we can use it right away to populate the default state value. - const findActiveRouteRoles = useCallback(() => { - // Starts with the widget roles and adds the others in. + useEffect(() => { const roles: string[] = []; - - // Route roles for (const match of matches) { if (isRoleRouteObject(match)) { for (const role of match.handle.roles) { @@ -21,13 +17,8 @@ const useActiveRouteRoleWatcher = () => { } } } - - return roles; + setActiveRouteRoles(roles); }, [matches]); - - useEffect(() => { - setActiveRouteRoles(findActiveRouteRoles()); - }, [matches, findActiveRouteRoles]); }; export default useActiveRouteRoleWatcher; diff --git a/runtime/slots/Slot.tsx b/runtime/slots/Slot.tsx index 425a5da7..8b2b04f8 100644 --- a/runtime/slots/Slot.tsx +++ b/runtime/slots/Slot.tsx @@ -1,4 +1,4 @@ -import { ComponentType, createElement, isValidElement, ReactNode } from 'react'; +import { ComponentType, createElement, isValidElement, memo, ReactNode } from 'react'; import DefaultSlotLayout from './layout/DefaultSlotLayout'; import { useLayoutForSlotId } from './layout/hooks'; import SlotContext from './SlotContext'; @@ -30,10 +30,13 @@ function SlotRenderer({ layout }: { layout: ComponentType | ReactNode }) { return <>{layoutElement}; } -export default function Slot({ id, idAliases, children, layout = DefaultSlotLayout, ...props }: SlotProps) { +function Slot({ id, idAliases, children, layout = DefaultSlotLayout, ...props }: SlotProps) { return ( ); } + +/* memo so parent re-renders don't cascade through Slot when props are shallow-equal. */ +export default memo(Slot); diff --git a/runtime/slots/hooks.ts b/runtime/slots/hooks.ts index 3578a4a9..48cfddf9 100644 --- a/runtime/slots/hooks.ts +++ b/runtime/slots/hooks.ts @@ -1,33 +1,20 @@ -import { useContext, useEffect, useState } from 'react'; -import { useLocation } from 'react-router-dom'; +import { useContext, useMemo } from 'react'; -import { SlotOperation } from './types'; import { getSlotOperations } from './utils'; import SlotContext from './SlotContext'; import { createWidgetAppendOperation } from './widget'; /** - * The useSlotOperations hook will trigger re-renders when the slot configuration changes. - * It is a fundamental hook that is used by many of the others to ensure they're using up-to-date - * config as it changes. + * Resolves the operations registered for a given slot id (plus any aliases), + * prepended with a default-content operation built from the slot's children. */ export function useSlotOperations(id: string) { const { children, idAliases } = useSlotContext(); - const location = useLocation(); - const [operations, setOperations] = useState([]); - useEffect(() => { - // Setting default content has to happen inside `useEffect()` so that re-renders only happen - // when [children] props change. This avoids an endless render loop. After all, the whole - // point of a slot is to modify its children via slot operations. + return useMemo(() => { const defaultOperation = createWidgetAppendOperation('defaultContent', id, children); - setOperations(getSlotOperations([id, ...(idAliases ?? [])], defaultOperation)); - - // We depend on [location] to force re-renders on navigation. This guarantees changes in active - // roles (and thus, changes in what conditional widgets are shown) properly. - }, [id, children, idAliases, location]); - - return operations; + return getSlotOperations([id, ...(idAliases ?? [])], defaultOperation); + }, [id, children, idAliases]); } export function useSlotContext() { diff --git a/runtime/slots/layout/hooks.ts b/runtime/slots/layout/hooks.ts index f6816b07..2573687c 100644 --- a/runtime/slots/layout/hooks.ts +++ b/runtime/slots/layout/hooks.ts @@ -1,4 +1,5 @@ -import { ComponentType, ReactNode, useEffect, useState } from 'react'; +import { ComponentType, ReactNode, useMemo } from 'react'; +import useActiveRoles from '../../react/hooks/useActiveRoles'; import { useSlotContext, useSlotOperations } from '../hooks'; import { isSlotOperationConditionSatisfied } from '../utils'; import { LayoutComponentProps, LayoutElementProps, LayoutOperation } from './types'; @@ -14,11 +15,12 @@ function hasLayoutElementProps(operation: LayoutOperation): operation is (Layout export function useLayoutForSlotId(id: string) { const operations = useSlotOperations(id); - let layoutElement: ReactNode | ComponentType = null; + const activeRoles = useActiveRoles(); - for (const operation of operations) { - if (isSlotOperationConditionSatisfied(operation)) { - if (isLayoutReplaceOperation(operation)) { + return useMemo(() => { + let layoutElement: ReactNode | ComponentType = null; + for (const operation of operations) { + if (isSlotOperationConditionSatisfied(operation, activeRoles) && isLayoutReplaceOperation(operation)) { if (hasLayoutComponentProps(operation)) { layoutElement = operation.component; } else if (hasLayoutElementProps(operation)) { @@ -26,15 +28,14 @@ export function useLayoutForSlotId(id: string) { } } } - } - return layoutElement; + return layoutElement; + }, [operations, activeRoles]); } /** * useLayoutOptions iterates through the slot's operations to find any which are "layout * options" operations. It merges these into a single object and returns them - operations are - * merged in declaration order, meaning last one in wins. useLayoutOptions only triggers a - * re-render when the options change. + * merged in declaration order, meaning last one in wins. */ export function useLayoutOptions() { const { id } = useSlotContext(); @@ -43,23 +44,15 @@ export function useLayoutOptions() { export function useLayoutOptionsForId(id: string) { const operations = useSlotOperations(id); - const [options, setOptions] = useState>({}); + const activeRoles = useActiveRoles(); - useEffect(() => { - const findOptions = () => { - let nextOptions: Record = {}; - for (const operation of operations) { - if (isSlotOperationConditionSatisfied(operation)) { - if (isLayoutOptionsOperation(operation)) { - nextOptions = { ...nextOptions, ...operation.options }; - } - } + return useMemo(() => { + let options: Record = {}; + for (const operation of operations) { + if (isSlotOperationConditionSatisfied(operation, activeRoles) && isLayoutOptionsOperation(operation)) { + options = { ...options, ...operation.options }; } - return nextOptions; - }; - - setOptions(findOptions()); - }, [operations]); - - return options; + } + return options; + }, [operations, activeRoles]); } diff --git a/runtime/slots/utils.ts b/runtime/slots/utils.ts index c92861e9..b5e33b2d 100644 --- a/runtime/slots/utils.ts +++ b/runtime/slots/utils.ts @@ -1,5 +1,5 @@ import { getAuthenticatedUser } from '../auth'; -import { getActiveRoles, getSiteConfig } from '../config'; +import { getSiteConfig } from '../config'; import { SlotOperation } from './types'; export function getSlotOperations(ids: string[], defaultOperation?: SlotOperation) { @@ -25,7 +25,7 @@ export function getSlotOperations(ids: string[], defaultOperation?: SlotOperatio return ops; } -export function isSlotOperationConditionSatisfied(operation: SlotOperation) { +export function isSlotOperationConditionSatisfied(operation: SlotOperation, activeRoles: string[]) { const { condition } = operation; if (condition?.authenticated !== undefined) { const isAuthenticated = getAuthenticatedUser() !== null; @@ -36,8 +36,6 @@ export function isSlotOperationConditionSatisfied(operation: SlotOperation) { } if (condition?.active !== undefined || condition?.inactive !== undefined) { - const activeRoles: string[] = getActiveRoles(); - if (condition?.active !== undefined) { let activeConditionRoleFound = false; for (const conditionRole of condition.active) { diff --git a/runtime/slots/widget/hooks.ts b/runtime/slots/widget/hooks.ts index 2c8b2465..46df968d 100644 --- a/runtime/slots/widget/hooks.ts +++ b/runtime/slots/widget/hooks.ts @@ -1,4 +1,5 @@ -import { useContext, useEffect, useState } from 'react'; +import { useContext, useMemo } from 'react'; +import useActiveRoles from '../../react/hooks/useActiveRoles'; import { useSlotContext, useSlotOperations } from '../hooks'; import { isSlotOperationConditionSatisfied } from '../utils'; import { WidgetList, WidgetOperation } from './types'; @@ -18,31 +19,26 @@ export function useWidgetsForId(id: string, componentProps?: Record([]); - - useEffect(() => { - const filteredOperations = operations.filter((operation): operation is WidgetOperation => { - return isWidgetOperation(operation) && isSlotOperationConditionSatisfied(operation); - }); - - setWidgetOperations(filteredOperations); - }, [operations]); - - return widgetOperations; + const activeRoles = useActiveRoles(); + + return useMemo( + () => operations.filter((operation): operation is WidgetOperation => ( + isWidgetOperation(operation) && isSlotOperationConditionSatisfied(operation, activeRoles) + )), + [operations, activeRoles], + ); } export function useSortedWidgetOperations(id: string) { const operations = useWidgetOperations(id); - const [sortedOperations, setSortedOperations] = useState([]); - useEffect(() => { + return useMemo(() => ( // This sorts widget operations in an order that guarantees that any 'related' widgets // needed by relatively positioned operations (INSERT_AFTER, INSERT_BEFORE) should already exist // by the time the relative operations are evaluated. It means the declaration order of // operations in SiteConfig does not prevent an operation from interacting with a widget that was // declared 'later'. - const sortedOperations = operations.sort((a: WidgetOperation, b: WidgetOperation) => { - // If both operations are widget operations, there are special sorting rules. + [...operations].sort((a: WidgetOperation, b: WidgetOperation) => { const aAbsolute = isWidgetAbsoluteOperation(a); const bAbsolute = isWidgetAbsoluteOperation(b); if (aAbsolute && bAbsolute) { @@ -58,21 +54,15 @@ export function useSortedWidgetOperations(id: string) { return 1; } } - return 0; - }); - - setSortedOperations(sortedOperations); - }, [operations]); - - return sortedOperations; + }) + ), [operations]); } /** * useWidgetOptions iterates through the slot's operations to find any that are "widget options" * operations specific to this widget. It merges these into a single object and returns them - - * operations are merged in declaration order, meaning last one in wins. useWidgetOptions only - * triggers a re-render when the options change. + * operations are merged in declaration order, meaning last one in wins. */ export function useWidgetOptions() { const { slotId, widgetId } = useContext(WidgetContext); @@ -82,24 +72,13 @@ export function useWidgetOptions() { export function useWidgetOptionsForId(slotId: string, widgetId: string) { const operations = useWidgetOperations(slotId); - const [options, setOptions] = useState>({}); - useEffect(() => { - const findOptions = () => { - let nextOptions: Record = {}; - for (const operation of operations) { - if (isSlotOperationConditionSatisfied(operation)) { - if (isWidgetOptionsOperation(operation)) { - if (operation.relatedId === widgetId) { - nextOptions = { ...nextOptions, ...operation.options }; - } - } - } + return useMemo(() => { + let options: Record = {}; + for (const operation of operations) { + if (isWidgetOptionsOperation(operation) && operation.relatedId === widgetId) { + options = { ...options, ...operation.options }; } - return nextOptions; - }; - - setOptions(findOptions()); + } + return options; }, [widgetId, operations]); - - return options; } diff --git a/runtime/slots/widget/utils.test.tsx b/runtime/slots/widget/utils.test.tsx index be0de4dc..7fbe5ba0 100644 --- a/runtime/slots/widget/utils.test.tsx +++ b/runtime/slots/widget/utils.test.tsx @@ -8,11 +8,6 @@ jest.mock('./WidgetProvider', () => ({ default: ({ children }: { children: React.ReactNode }) => <>{children}, })); -// Mock condition checking to always pass. -jest.mock('../utils', () => ({ - isSlotOperationConditionSatisfied: () => true, -})); - const slotId = 'test-slot'; function makeAppendOp(id: string, label: string, role?: string) { diff --git a/runtime/slots/widget/utils.tsx b/runtime/slots/widget/utils.tsx index eaaf13fc..ad1c257e 100644 --- a/runtime/slots/widget/utils.tsx +++ b/runtime/slots/widget/utils.tsx @@ -1,6 +1,5 @@ import { ReactNode } from 'react'; import { SlotOperation } from '../types'; -import { isSlotOperationConditionSatisfied } from '../utils'; import { IFrameWidget } from './iframe'; import { IdentifiedWidget, WidgetAbsoluteOperation, WidgetAppendOperation, WidgetComponentProps, WidgetElementProps, WidgetIdentityProps, WidgetIFrameProps, WidgetInsertAfterOperation, WidgetInsertBeforeOperation, WidgetList, WidgetOperation, WidgetOperationTypes, WidgetOptionsOperation, WidgetPrependOperation, WidgetRemoveOperation, WidgetRendererOperation, WidgetRendererProps, WidgetReplaceOperation } from './types'; import WidgetProvider from './WidgetProvider'; @@ -205,20 +204,18 @@ export function createWidgets(operations: WidgetOperation[], componentProps?: Re const identifiedWidgets: IdentifiedWidget[] = []; for (const operation of operations) { - if (isSlotOperationConditionSatisfied(operation)) { - if (isWidgetAppendOperation(operation)) { - appendWidget(operation, identifiedWidgets, componentProps); - } else if (isWidgetPrependOperation(operation)) { - prependWidget(operation, identifiedWidgets, componentProps); - } else if (isWidgetInsertAfterOperation(operation)) { - insertAfterWidget(operation, identifiedWidgets, componentProps); - } else if (isWidgetInsertBeforeOperation(operation)) { - insertBeforeWidget(operation, identifiedWidgets, componentProps); - } else if (isWidgetReplaceOperation(operation)) { - replaceWidget(operation, identifiedWidgets, componentProps); - } else if (isWidgetRemoveOperation(operation)) { - removeWidget(operation, identifiedWidgets); - } + if (isWidgetAppendOperation(operation)) { + appendWidget(operation, identifiedWidgets, componentProps); + } else if (isWidgetPrependOperation(operation)) { + prependWidget(operation, identifiedWidgets, componentProps); + } else if (isWidgetInsertAfterOperation(operation)) { + insertAfterWidget(operation, identifiedWidgets, componentProps); + } else if (isWidgetInsertBeforeOperation(operation)) { + insertBeforeWidget(operation, identifiedWidgets, componentProps); + } else if (isWidgetReplaceOperation(operation)) { + replaceWidget(operation, identifiedWidgets, componentProps); + } else if (isWidgetRemoveOperation(operation)) { + removeWidget(operation, identifiedWidgets); } } diff --git a/runtime/utils.js b/runtime/utils.js index ee1a1067..cc76adf8 100644 --- a/runtime/utils.js +++ b/runtime/utils.js @@ -3,8 +3,8 @@ * * @module Utilities */ -import camelCase from 'lodash.camelcase'; -import snakeCase from 'lodash.snakecase'; +import camelCase from 'lodash/camelCase'; +import snakeCase from 'lodash/snakeCase'; /** * This is the underlying function used by camelCaseObject, snakeCaseObject, and convertKeyNames diff --git a/tools/cli/utils/translations/prepare.ts b/tools/cli/utils/translations/prepare.ts index 1829086e..84048ba9 100644 --- a/tools/cli/utils/translations/prepare.ts +++ b/tools/cli/utils/translations/prepare.ts @@ -1,5 +1,5 @@ import fs from 'fs'; -import camelCase from 'lodash.camelcase'; +import camelCase from 'lodash/camelCase'; import path from 'path'; import { generateMessagesObject, writeMessagesObjectToFile, type MessagesObject } from './messagesObject';