Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 93 additions & 1 deletion src/engine/MacroChoiceEngine.entry.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { MacroChoiceEngine } from "./MacroChoiceEngine";
import type QuickAdd from "../main";
import type { IChoiceExecutor } from "../IChoiceExecutor";
Expand Down Expand Up @@ -211,6 +211,98 @@ describe("MacroChoiceEngine user script entry handling", () => {
});
});

describe("MacroChoiceEngine user script variable propagation", () => {
const app = {} as App;
const plugin = {} as unknown as QuickAdd;

let choiceExecutor: IChoiceExecutor;
let variables: Map<string, unknown>;
let macroChoice: IMacroChoice;
let logs: Array<number | undefined>;
let getApiSpy: ReturnType<typeof vi.spyOn>;

beforeEach(() => {
vi.clearAllMocks();
mockGetUserScript.mockReset();
mockInitializeUserScriptSettings.mockReset();
mockSuggest.mockReset();

getApiSpy = vi
.spyOn(QuickAddApi, "GetApi")
.mockReturnValue({} as unknown as ReturnType<typeof QuickAddApi.GetApi>);

logs = [];
variables = new Map<string, unknown>();
choiceExecutor = {
execute: vi.fn(),
variables,
};

const makeScript = (index: number): IUserScript => ({
id: `script-${index}`,
name: `Script ${index}`,
type: CommandType.UserScript,
path: `script-${index}.js`,
settings: {},
});

macroChoice = {
id: "macro-sequence",
name: "Macro sequence",
type: "Macro",
command: false,
runOnStartup: false,
macro: {
id: "macro-sequence",
name: "Macro sequence",
commands: [
makeScript(1),
makeScript(2),
makeScript(3),
makeScript(4),
],
} as IMacro,
};

mockGetUserScript.mockImplementation((command: IUserScript) => {
const nextValueByPath: Record<string, number> = {
"script-1.js": 1,
"script-2.js": 2,
"script-3.js": 3,
"script-4.js": 3,
};

const nextValue = nextValueByPath[command.path ?? ""] ?? 0;

return Promise.resolve(async (params: { variables: Record<string, unknown> }) => {
logs.push(params.variables.target as number | undefined);
if (nextValue !== 0) {
params.variables.target = nextValue;
}
});
});
});

afterEach(() => {
getApiSpy.mockRestore();
});

it("propagates variable mutations across sequential user scripts", async () => {
const engine = new MacroChoiceEngine(
app,
plugin,
macroChoice,
choiceExecutor,
variables,
);

await engine.run();

expect(logs).toEqual([undefined, 1, 2, 3]);
expect(choiceExecutor.variables.get("target")).toBe(3);
});
});

describe("MacroChoiceEngine choice command cancellation", () => {
const app = {} as App;
let plugin: QuickAdd & { getChoiceById: ReturnType<typeof vi.fn> };
Expand Down
27 changes: 7 additions & 20 deletions src/engine/MacroChoiceEngine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import type { ScriptCondition } from "../types/macros/Conditional/types";
import { evaluateCondition } from "./helpers/conditionalEvaluator";
import { handleMacroAbort } from "../utils/macroAbortHandler";
import { buildOpenFileOptions } from "./helpers/openFileOptions";
import { createVariablesProxy } from "../utils/variablesProxy";

type ConditionalScriptRunner = () => Promise<unknown>;

Expand Down Expand Up @@ -93,19 +94,20 @@ export class MacroChoiceEngine extends QuickAddChoiceEngine {
this.macro = choice?.macro;
this.choiceExecutor = choiceExecutor;
this.preloadedUserScripts = preloadedUserScripts ?? new Map();
const sharedVariables =
variables ??
this.choiceExecutor.variables ??
new Map<string, unknown>();
this.choiceExecutor.variables = sharedVariables;
this.params = {
app: this.app,
quickAddApi: QuickAddApi.GetApi(app, plugin, choiceExecutor),
variables: {},
variables: createVariablesProxy(this.choiceExecutor.variables),
obsidian,
abort: (message?: string) => {
throw new MacroAbortError(message);
},
};

variables?.forEach((value, key) => {
this.params.variables[key] = value;
});
}

async run(): Promise<void> {
Expand Down Expand Up @@ -151,14 +153,6 @@ export class MacroChoiceEngine extends QuickAddChoiceEngine {
if (command?.type === CommandType.Conditional) {
await this.executeConditional(command as IConditionalCommand);
}

this.pullExecutorVariablesIntoParams();
Object.keys(this.params.variables).forEach((key) => {
this.choiceExecutor.variables.set(
key,
this.params.variables[key]
);
});
}
} catch (error) {
if (
Expand Down Expand Up @@ -462,7 +456,6 @@ export class MacroChoiceEngine extends QuickAddChoiceEngine {
}

private async executeConditional(command: IConditionalCommand) {
this.pullExecutorVariablesIntoParams();
const shouldRunThenBranch = await evaluateCondition(command.condition, {
variables: this.params.variables,
evaluateScriptCondition: async (condition: ScriptCondition) =>
Expand All @@ -489,12 +482,6 @@ export class MacroChoiceEngine extends QuickAddChoiceEngine {
this.output = value;
}

private pullExecutorVariablesIntoParams() {
this.choiceExecutor.variables.forEach((value, key) => {
this.params.variables[key] = value;
});
}

private async evaluateScriptCondition(
condition: ScriptCondition
): Promise<boolean> {
Expand Down
5 changes: 0 additions & 5 deletions src/engine/SingleMacroEngine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -288,11 +288,6 @@ export class SingleMacroEngine {
engine: MacroChoiceEngine,
settings: Record<string, unknown>,
): Promise<unknown> {
// Ensure params reflect latest shared variables before executing
this.choiceExecutor.variables.forEach((value, key) => {
engine.params.variables[key] = value;
});

if (typeof member === "function") {
return await (
member as (
Expand Down
27 changes: 27 additions & 0 deletions src/utils/variablesProxy.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { describe, expect, it } from "vitest";
import { createVariablesProxy } from "./variablesProxy";

describe("createVariablesProxy", () => {
it("reads and writes through to the backing map", () => {
const backing = new Map<string, unknown>([["a", 1]]);
const proxy = createVariablesProxy(backing);

expect(proxy.a).toBe(1);
proxy.b = 2;
expect(backing.get("b")).toBe(2);

delete proxy.a;
expect(backing.has("a")).toBe(false);
});

it("enumerates keys like a plain object", () => {
const backing = new Map<string, unknown>([
["foo", "bar"],
["num", 3],
]);
const proxy = createVariablesProxy(backing);

expect(Object.keys(proxy)).toEqual(["foo", "num"]);
expect(JSON.stringify(proxy)).toBe('{"foo":"bar","num":3}');
});
});
39 changes: 39 additions & 0 deletions src/utils/variablesProxy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/**
* Creates a plain-object style proxy over a Map so scripts can read/write
* `params.variables.foo` while the underlying source of truth remains the Map.
*/
export function createVariablesProxy(
store: Map<string, unknown>
): Record<string, unknown> {
const target = Object.create(null);

return new Proxy(target, {
get(_t, prop: string | symbol) {
if (typeof prop !== "string") return undefined;
return store.get(prop);
},
set(_t, prop: string | symbol, value) {
if (typeof prop !== "string") return true;
store.set(prop, value);
return true;
},
deleteProperty(_t, prop: string | symbol) {
if (typeof prop !== "string") return true;
store.delete(prop);
return true;
},
has(_t, prop: string | symbol) {
return typeof prop === "string" && store.has(prop);
},
ownKeys() {
return Array.from(store.keys());
},
getOwnPropertyDescriptor(_t, prop: string | symbol) {
if (typeof prop !== "string" || !store.has(prop)) return undefined;
return {
configurable: true,
enumerable: true,
};
},
});
}