Skip to content

Commit 5903de7

Browse files
seefeldbclaudebfollington
authored
Fix reactive dependencies not triggering when intermediate path appears (commontoolsinc#2160)
* Fix reactive dependencies not triggering when intermediate path appears When watching a path like ["a", "b", "c"], the algorithm would not trigger if an intermediate segment appeared. For example: - Before: { a: {} } - path a.b.c is "unreachable" (b doesn't exist) - After: { a: { b: { x: 1 } } } - b exists but c doesn't Both cases resolve to `undefined`, so no change was detected. But this is a structural change that should trigger subscribers, because the path became "more reachable". The fix distinguishes three cases: 1. Both paths reachable → compare actual values (original behavior) 2. Reachability changed → trigger (NEW) 3. Neither reachable but different traversal depth → trigger (NEW) This fixes issues where handlers watching paths through dynamically appearing keys (like internal.__#0.allCharms) would not fire on first load. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Format pass --------- Co-authored-by: Claude <[email protected]> Co-authored-by: Ben Follington <[email protected]>
1 parent 966859a commit 5903de7

File tree

2 files changed

+123
-9
lines changed

2 files changed

+123
-9
lines changed

packages/runner/src/reactive-dependencies.ts

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,6 @@ export function determineTriggeredActions(
106106
action,
107107
paths: paths.toReversed(),
108108
}));
109-
subscribers.sort((a, b) => comparePaths(a.paths[0], b.paths[0]));
110109

111110
if (startPath.length > 0) {
112111
// If we're starting from a specific path, filter the subscribers to only
@@ -160,15 +159,32 @@ export function determineTriggeredActions(
160159
}
161160
currentPath = targetPath;
162161

163-
// Now get the value at the path, `undefined` if that path doesn't exist
164-
const beforeValue = beforeLastObject + 1 >= targetPath.length
165-
? beforeValues[targetPath.length]
166-
: undefined;
167-
const afterValue = afterLastObject + 1 >= targetPath.length
168-
? afterValues[targetPath.length]
169-
: undefined;
162+
// Check if we could traverse far enough to reach the target path
163+
const beforeCanReach = beforeLastObject + 1 >= targetPath.length;
164+
const afterCanReach = afterLastObject + 1 >= targetPath.length;
165+
166+
// Determine if there was a change. We need to trigger if:
167+
// 1. Both paths are reachable and the values differ
168+
// 2. Reachability changed (one can reach, the other can't)
169+
// 3. Neither can reach, but the depth of reachability changed
170+
// (e.g., before we couldn't get past "a", now we can get to "a.b")
171+
let hasChanged: boolean;
172+
if (beforeCanReach && afterCanReach) {
173+
// Both reachable - compare actual values
174+
hasChanged = !deepEqual(
175+
beforeValues[targetPath.length],
176+
afterValues[targetPath.length],
177+
);
178+
} else if (beforeCanReach !== afterCanReach) {
179+
// Reachability changed - definitely a structural change
180+
hasChanged = true;
181+
} else {
182+
// Neither reachable - check if we can traverse to different depths
183+
// This detects when intermediate path segments appear/disappear
184+
hasChanged = beforeLastObject !== afterLastObject;
185+
}
170186

171-
if (!deepEqual(beforeValue, afterValue)) {
187+
if (hasChanged) {
172188
// If the value changed, trigger the actions
173189
triggeredActions.push(...current.map(({ action }) => action));
174190
} else {

packages/runner/test/reactive-dependencies.test.ts

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1613,6 +1613,104 @@ describe("determineTriggeredActions", () => {
16131613
});
16141614
});
16151615

1616+
describe("bug: action should trigger when intermediate path appears", () => {
1617+
it("triggers when intermediate path appears even if final value remains undefined", () => {
1618+
// Bug scenario:
1619+
// Action B watches ["a", "b", "c"]
1620+
// Before: { a: {} } - no 'b' key, so path a.b.c is "unreachable"
1621+
// After: { a: { b: { x: 1 } } } - 'b' exists but no 'c' key
1622+
//
1623+
// In both cases the final value at a.b.c is undefined, BUT:
1624+
// - Before: we couldn't traverse past 'a' because 'b' doesn't exist
1625+
// - After: we can traverse to 'b', but 'c' doesn't exist there
1626+
//
1627+
// This structural change should trigger the action, because the
1628+
// "reachability" of the path changed.
1629+
const actionB = createAction("actionB");
1630+
const dependencies = new Map<Action, SortedAndCompactPaths>([
1631+
[actionB, [["a", "b", "c"]]],
1632+
]);
1633+
1634+
const result = determineTriggeredActions(
1635+
dependencies,
1636+
{ a: {} },
1637+
{ a: { b: { x: 1 } } },
1638+
);
1639+
1640+
// Action B should trigger because the path structure changed
1641+
expect(result).toContain(actionB);
1642+
});
1643+
1644+
it("real-world case: triggers when __#0 key appears in internal", () => {
1645+
// Simplified version of the real bug from production
1646+
// Action B watches a path through __#0.allCharms
1647+
// Before: internal doesn't have __#0
1648+
// After: internal has __#0 (but no allCharms inside)
1649+
const actionA = createAction("actionA");
1650+
const actionB = createAction("actionB");
1651+
const actionC = createAction("actionC");
1652+
1653+
const dependencies = new Map<Action, SortedAndCompactPaths>([
1654+
[actionA, [
1655+
["value", "internal", "$NAME"],
1656+
["value", "internal", "$alias", "path"],
1657+
["value", "internal", "/", "link@1"],
1658+
["value", "internal", "cell", "/"],
1659+
]],
1660+
[actionB, [
1661+
["value", "internal", "$alias", "path"],
1662+
["value", "internal", "/", "link@1"],
1663+
["value", "internal", "__#0", "allCharms"],
1664+
["value", "internal", "cell", "/"],
1665+
]],
1666+
[actionC, [
1667+
["value", "internal", "$alias", "path"],
1668+
["value", "internal", "/", "link@1"],
1669+
["value", "internal", "__#0", "allCharms"],
1670+
["value", "internal", "cell", "/"],
1671+
]],
1672+
]);
1673+
1674+
const before = {
1675+
value: {
1676+
internal: {
1677+
backlinksIndex: { "/": { "link@1": { path: [], id: "of:123" } } },
1678+
"__#2": { "/": { "link@1": { path: [], id: "of:456" } } },
1679+
// Note: NO __#0 key
1680+
},
1681+
},
1682+
};
1683+
1684+
const after = {
1685+
value: {
1686+
internal: {
1687+
backlinksIndex: { "/": { "link@1": { path: [], id: "of:123" } } },
1688+
"__#2": { "/": { "link@1": { path: [], id: "of:456" } } },
1689+
// __#0 now exists! (but doesn't have allCharms)
1690+
"__#0": {
1691+
"/": {
1692+
"link@1": { path: [], id: "of:789", space: "did:key:abc" },
1693+
},
1694+
},
1695+
},
1696+
},
1697+
};
1698+
1699+
const result = determineTriggeredActions(
1700+
dependencies,
1701+
before as JSONValue,
1702+
after as JSONValue,
1703+
);
1704+
1705+
// Action B and C should trigger because __#0 appeared,
1706+
// even though allCharms is still undefined in both cases
1707+
expect(result).toContain(actionB);
1708+
expect(result).toContain(actionC);
1709+
// Action A should NOT trigger (none of its paths changed)
1710+
expect(result).not.toContain(actionA);
1711+
});
1712+
});
1713+
16161714
describe("complex scenarios", () => {
16171715
it("handles multiple actions with different nested dependencies", () => {
16181716
const action1 = createAction("action1");

0 commit comments

Comments
 (0)