repos / starfx

commit

783202c

parent

9f955d2

author

Vlad

date

2025-06-15 08:22:02 -0400 EDT

Refactor matcher for correct predicate and action handling (#62)

- Brand action creators using a cheap, non-enumerable _starfx property for precise detection.
- Refactor matcher logic to clearly separate branded action creators from custom predicates and all other cases.
- This ensures only the intended actions are matched, fixes the failing test, and makes the matcher safe for userland/test usage and advanced patterns.
- Additionally added matcher.test.ts to outline matcher behavior and edge cases, ensuring future changes remain safe.
- Also added index signature to ActionWithPayload to ensure that is compatible with react-redux UnknownAction type [type.ts]

 1@@ -64,7 +64,6 @@ export function* take(pattern: ActionPattern): Operation<Action> {
 2   for (const action of yield* each(fd)) {
 3     return action;
 4   }
 5-
 6   return { type: "take failed, this should not be possible" };
 7 }
 8 
 9@@ -141,6 +140,11 @@ export function createAction(actionType: string) {
10     payload,
11   });
12   fn.toString = () => actionType;
13-
14+  Object.defineProperty(fn, "_starfx", {
15+    value: true,
16+    enumerable: false,
17+    configurable: false,
18+    writable: false,
19+  });
20   return fn;
21 }

 1@@ -6,7 +6,7 @@ type Predicate<Guard extends AnyAction = AnyAction> = (
 2   action: Guard,
 3 ) => boolean;
 4 type StringableActionCreator<A extends AnyAction = AnyAction> = {
 5-  (...args: unknown[]): A;
 6+  (...args: any[]): A;
 7   toString(): string;
 8 };
 9 type SubPattern<Guard extends AnyAction = AnyAction> =
10@@ -27,11 +27,16 @@ function isThunk(fn: any): boolean {
11   return (
12     typeof fn === "function" &&
13     typeof fn.run === "function" &&
14+    typeof fn.use === "function" &&
15     typeof fn.name === "string" &&
16-    typeof fn.key === "string" &&
17     typeof fn.toString === "function"
18   );
19 }
20+
21+function isActionCreator(fn: any): boolean {
22+  return !!fn && fn._starfx === true;
23+}
24+
25 export function matcher(pattern: ActionPattern): Predicate {
26   if (pattern === "*") {
27     return (input) => !!input;
28@@ -45,14 +50,17 @@ export function matcher(pattern: ActionPattern): Predicate {
29     return (input) => pattern.some((p) => matcher(p)(input));
30   }
31 
32-  // detects thunk action creators
33   if (isThunk(pattern)) {
34     return (input) => pattern.toString() === input.type;
35   }
36 
37-  if (typeof pattern === "function") {
38+  if (typeof pattern === "function" && !isActionCreator(pattern)) {
39     return (input) => pattern(input) as boolean;
40   }
41 
42+  if (isActionCreator(pattern)) {
43+    return (input: AnyAction) => pattern.toString() === input.type;
44+  }
45+
46   throw new Error("invalid pattern");
47 }

 1@@ -10,3 +10,13 @@ test("return object with type", () => {
 2   const undo = createAction("UNDO");
 3   expect(undo()).toEqual({ type: "UNDO", payload: undefined });
 4 });
 5+
 6+test("createAction with object type should be compatible with UnknownAction", () => {
 7+  expect.assertions(1);
 8+  const emptyAction = createAction<object>("EMPTY_ACTION");
 9+  const action = emptyAction({});
10+
11+  // This should compile without TypeScript errors - testing the index signature
12+  const hasIndexSignature = (action as any).someRandomProperty === undefined;
13+  expect(hasIndexSignature).toBe(true);
14+});

  1@@ -0,0 +1,228 @@
  2+import {
  3+  type ThunkCtx,
  4+  createAction,
  5+  createThunks,
  6+  sleep,
  7+  takeLatest,
  8+} from "../index.js";
  9+import { matcher } from "../matcher.js";
 10+import { createStore } from "../store/index.js";
 11+import { expect, test } from "../test.js";
 12+
 13+test("true", () => {
 14+  expect(true).toBe(true);
 15+});
 16+
 17+// The main thing
 18+test("createAction should not match all actions", async () => {
 19+  expect.assertions(1);
 20+
 21+  const store = createStore({ initialState: {} });
 22+  const matchedActions: string[] = [];
 23+
 24+  const testAction = createAction("test/action");
 25+
 26+  function* testFn(action: any) {
 27+    matchedActions.push(action.type);
 28+  }
 29+
 30+  function* root() {
 31+    yield* takeLatest(testAction, testFn);
 32+  }
 33+  const task = store.run(root);
 34+
 35+  store.dispatch({ type: "test/action", payload: { MenuOpened: "test" } });
 36+  store.dispatch({ type: "store", payload: { something: "else" } });
 37+  store.dispatch({ type: "other/action", payload: { data: "test" } });
 38+
 39+  await new Promise((resolve) => setTimeout(resolve, 100));
 40+  expect(matchedActions).toEqual(["test/action"]);
 41+  await task.halt();
 42+});
 43+
 44+test("matcher should correctly identify createAction functions", () => {
 45+  expect.assertions(2);
 46+
 47+  const actionCreator = createAction("test/action");
 48+  const match = matcher(actionCreator);
 49+  expect(match({ type: "test/action", payload: {} })).toBe(true);
 50+  expect(match({ type: "other/action", payload: {} })).toBe(false);
 51+});
 52+
 53+test("typed createAction should work with takeLatest without type casting", async () => {
 54+  expect.assertions(1);
 55+
 56+  const store = createStore({ initialState: {} });
 57+  const matchedActions: string[] = [];
 58+
 59+  //typed action creator - this should work without 'as any'
 60+  const typedAction = createAction<{ MenuOpened: string }>("TYPED_ACTION");
 61+
 62+  function* handler(action: any) {
 63+    matchedActions.push(action.type);
 64+  }
 65+
 66+  function* root() {
 67+    // Should compile without TypeScript errors - no 'as any' needed
 68+    yield* takeLatest(typedAction, handler);
 69+  }
 70+
 71+  const task = store.run(root);
 72+
 73+  // dispatch the typed action
 74+  store.dispatch(typedAction({ MenuOpened: "settings" }));
 75+
 76+  // unrelated actions that should NOT trigger handler
 77+  store.dispatch({ type: "RANDOM_ACTION", payload: { data: "test" } });
 78+
 79+  await new Promise((resolve) => setTimeout(resolve, 100));
 80+  expect(matchedActions).toEqual(["TYPED_ACTION"]);
 81+
 82+  await task.halt();
 83+});
 84+
 85+test("should correctly identify starfx thunk as a thunk", async () => {
 86+  expect.assertions(6);
 87+
 88+  const thunks = createThunks();
 89+  thunks.use(thunks.routes());
 90+
 91+  const store = createStore({
 92+    initialState: {
 93+      users: {},
 94+    },
 95+  });
 96+  store.run(thunks.bootup);
 97+
 98+  const myThunk = thunks.create("my-thunk", function* (_ctx, next) {
 99+    yield* next();
100+  });
101+
102+  // Test that thunk has the expected properties for isThunk detection
103+  expect(typeof myThunk.run).toBe("function");
104+  expect(typeof myThunk.use).toBe("function");
105+  expect(typeof myThunk.name).toBe("string");
106+  expect(typeof myThunk.toString).toBe("function");
107+
108+  // Verify it does NOT have the _starfx property (that's for action creators)
109+  expect((myThunk as any)._starfx).toBeUndefined();
110+
111+  // Verify it does NOT have a key property directly on the function
112+  expect(typeof (myThunk as any).key).toBe("undefined");
113+
114+  await new Promise((resolve) => setTimeout(resolve, 10));
115+});
116+
117+test("matcher should correctly identify thunk functions", async () => {
118+  expect.assertions(3);
119+
120+  const thunks = createThunks();
121+  thunks.use(thunks.routes());
122+
123+  const store = createStore({
124+    initialState: {
125+      users: {},
126+    },
127+  });
128+  store.run(thunks.bootup);
129+
130+  const myThunk = thunks.create("my-thunk", function* (_ctx, next) {
131+    yield* next();
132+  });
133+
134+  // Test that the matcher correctly identifies the thunk
135+  const match = matcher(myThunk);
136+  expect(match({ type: "my-thunk", payload: {} })).toBe(true);
137+  expect(match({ type: "other-action", payload: {} })).toBe(false);
138+
139+  // Verify the thunk's toString method returns the correct type
140+  expect(myThunk.toString()).toBe("my-thunk");
141+
142+  await new Promise((resolve) => setTimeout(resolve, 10));
143+});
144+
145+// the bug that determined we needed to write this matcher
146+test("some bug: createAction incorrectly matching all actions", async () => {
147+  expect.assertions(1);
148+
149+  const store = createStore({ initialState: {} });
150+  const matchedActions: string[] = [];
151+
152+  const testAction = createAction<{ MenuOpened: any }>("ACTION");
153+
154+  // Create a saga that should only respond to this specific action
155+  function* testFn(action: any) {
156+    matchedActions.push(action.type);
157+    yield* sleep(1);
158+  }
159+
160+  function* root() {
161+    yield* takeLatest(testAction, testFn);
162+  }
163+
164+  const task = store.run(root);
165+
166+  store.dispatch(testAction({ MenuOpened: "first" }));
167+  store.dispatch({ type: "store", payload: { something: "else" } });
168+  store.dispatch({ type: "other/action", payload: { data: "test" } });
169+
170+  await new Promise((resolve) => setTimeout(resolve, 100));
171+
172+  expect(matchedActions).toEqual(["ACTION"]);
173+
174+  await task.halt();
175+});
176+
177+test("should show the difference between createAction and thunk properties", () => {
178+  expect.assertions(8);
179+
180+  // starfx createAction
181+  const actionCreator = createAction<{ test: string }>("test/action");
182+
183+  // starfx thunk
184+  const thunks = createThunks();
185+  const myThunk = thunks.create("test-thunk", function* (_ctx: ThunkCtx, next) {
186+    yield* next();
187+  });
188+
189+  // Check properties of createAction
190+  expect(typeof actionCreator).toBe("function");
191+  expect(typeof actionCreator.toString).toBe("function");
192+  expect(actionCreator.toString()).toBe("test/action");
193+  expect(typeof (actionCreator as any).run).toBe("undefined"); // createAction doesn't have run
194+
195+  // Check properties of thunk
196+  expect(typeof myThunk).toBe("function");
197+  expect(typeof myThunk.toString).toBe("function");
198+  expect(typeof myThunk.run).toBe("function"); // thunk has run
199+  expect(typeof myThunk.name).toBe("string"); // actionFn
200+});
201+
202+test("debug: what path does createAction take in matcher", async () => {
203+  expect.assertions(7);
204+
205+  const actionCreator = createAction<{ test: string }>("test/action");
206+
207+  // Check that createAction has the _starfx branding property
208+  expect((actionCreator as any)._starfx).toBe(true);
209+
210+  // Verify it's NOT identified as a thunk (should fail isThunk checks)
211+  const hasRun = typeof (actionCreator as any).run === "function";
212+  const hasUse = typeof (actionCreator as any).use === "function";
213+  const hasKey = typeof (actionCreator as any).key === "string";
214+  expect(hasRun).toBe(false);
215+  expect(hasUse).toBe(false);
216+  expect(hasKey).toBe(false);
217+
218+  // Verify it has the properties needed for isActionCreator path
219+  const hasToString = typeof actionCreator.toString === "function";
220+  const isFunction = typeof actionCreator === "function";
221+  expect(hasToString).toBe(true);
222+  expect(isFunction).toBe(true);
223+
224+  // Most importantly: verify the matcher correctly identifies it as an action creator
225+  const match = matcher(actionCreator);
226+  expect(match({ type: "test/action", payload: { test: "value" } })).toBe(true);
227+
228+  await new Promise((resolve) => setTimeout(resolve, 10));
229+});

 1@@ -42,6 +42,7 @@ export interface Payload<P = any> {
 2 
 3 export interface Action {
 4   type: string;
 5+  [extraProps: string]: any;
 6 }
 7 
 8 export type ActionFn = () => { toString: () => string };
 9@@ -52,6 +53,7 @@ export interface AnyAction extends Action {
10   payload?: any;
11   meta?: any;
12   error?: boolean;
13+  [extraProps: string]: any;
14 }
15 
16 export interface ActionWithPayload<P> extends AnyAction {