-
Notifications
You must be signed in to change notification settings - Fork 277
O3-4931: Add ability to evaluate expressions in extensions #1422
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
4f77510
ac7d28b
c9b7944
4dfe696
52f608a
ef54a03
570cb89
f411cec
0132e1a
dbeb78b
3145e20
8a7dd13
ceee67a
2476598
b501c35
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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)) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These comparison guards should mean that we're calling |
||
| 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, | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One issue here is that we didn't prevent attempted evaluation of an empty string |
||
| 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 = {}; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be roughly equivalent to what was happening before, albeit with less chaining.