Skip to content
Open
Show file tree
Hide file tree
Changes from 9 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
27 changes: 27 additions & 0 deletions .changeset/lower-activity-step-actions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
"@stackflow/core": minor
"@stackflow/plugin-history-sync": minor
---

Allow step actions to target lower activities with targetActivityId

**Core Changes:**
- Step actions (stepPush, stepPop, stepReplace) can now target any activity in the stack using `targetActivityId` option
- Previously only the top activity could be targeted
- Enables modifying lower activity steps when popping current activity

**History Sync Plugin Changes:**
- Added intelligent synchronization when navigating to lower activities with removed steps
- When navigating back to a step that was removed via `stepPop`, automatically skips the removed entry
- Uses `history.back()` to navigate to the correct remaining step

**Use Case:**
```typescript
// Remove a step from a lower activity while at top activity
actions.stepPop({ targetActivityId: lowerActivityId });

// When user navigates back, the removed step entry is automatically skipped
history.back(); // Skips removed step, lands on the correct step
```

This is backward compatible - existing code works without changes.
42 changes: 23 additions & 19 deletions core/src/activity-utils/findTargetActivityIndices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,30 +77,34 @@ export function findTargetActivityIndices(
}
case "StepPushed":
case "StepReplaced": {
const latestActivity = findLatestActiveActivity(activities);
let targetActivity: Activity | undefined;

if (latestActivity) {
if (
event.targetActivityId &&
event.targetActivityId !== latestActivity.id
) {
break;
}
targetActivities.push(activities.indexOf(latestActivity));
if (event.targetActivityId) {
targetActivity = activities.find(
(activity) => activity.id === event.targetActivityId,
);
} else {
targetActivity = findLatestActiveActivity(activities);
}

if (targetActivity) {
targetActivities.push(activities.indexOf(targetActivity));
}
break;
}
case "StepPopped": {
const latestActivity = findLatestActiveActivity(activities);

if (latestActivity && latestActivity.steps.length > 1) {
if (
event.targetActivityId &&
event.targetActivityId !== latestActivity.id
) {
break;
}
targetActivities.push(activities.indexOf(latestActivity));
let targetActivity: Activity | undefined;

if (event.targetActivityId) {
targetActivity = activities.find(
(activity) => activity.id === event.targetActivityId,
);
} else {
targetActivity = findLatestActiveActivity(activities);
}

if (targetActivity && targetActivity.steps.length > 1) {
targetActivities.push(activities.indexOf(targetActivity));
}

break;
Expand Down
208 changes: 205 additions & 3 deletions core/src/aggregate.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type {
PoppedEvent,
PushedEvent,
ReplacedEvent,
StepPoppedEvent,
StepPushedEvent,
StepReplacedEvent,
} from "./event-types";
Expand Down Expand Up @@ -3974,7 +3975,7 @@ test("aggregate - After Push > Push > Pop > Replace, first pushed activity shoul
});
});

test("aggregate - StepPushedEvent must be ignored when top activity is not target activity", () => {
test("aggregate - StepPushedEvent can target lower activity with targetActivityId", () => {
const t = nowTime();

let pushedEvent1: PushedEvent;
Expand Down Expand Up @@ -4002,7 +4003,7 @@ test("aggregate - StepPushedEvent must be ignored when top activity is not targe
})),
(stepPushedEvent = makeEvent("StepPushed", {
stepId: "s1",
stepParams: {},
stepParams: { tab: "profile" },
targetActivityId: pushedEvent1.activityId,
eventDate: enoughPastTime(),
})),
Expand All @@ -4016,14 +4017,21 @@ test("aggregate - StepPushedEvent must be ignored when top activity is not targe
id: "A",
name: "sample",
transitionState: "enter-done",
params: {},
params: { tab: "profile" },
steps: [
{
id: "A",
params: {},
enteredBy: pushedEvent1,
zIndex: 0,
},
{
id: "s1",
params: { tab: "profile" },
enteredBy: stepPushedEvent,
zIndex: 0,
hasZIndex: false,
},
],
enteredBy: pushedEvent1,
isActive: false,
Expand Down Expand Up @@ -4333,3 +4341,197 @@ test("aggregate - StepPushedEvent에 hasZIndex 필드가 true이면, Step에 zIn
events,
});
});

test("aggregate - StepReplacedEvent can target lower activity with targetActivityId", () => {
const t = nowTime();

let pushedEvent1: PushedEvent;
let pushedEvent2: PushedEvent;
let stepReplacedEvent: StepReplacedEvent;

const events = [
initializedEvent({
transitionDuration: 300,
}),
registeredEvent({
activityName: "sample",
}),
(pushedEvent1 = makeEvent("Pushed", {
activityId: "A",
activityName: "sample",
activityParams: {},
eventDate: enoughPastTime(),
})),
(pushedEvent2 = makeEvent("Pushed", {
activityId: "B",
activityName: "sample",
activityParams: {},
eventDate: enoughPastTime(),
})),
(stepReplacedEvent = makeEvent("StepReplaced", {
stepId: "s1",
stepParams: { newParam: "value" },
targetActivityId: pushedEvent1.activityId,
eventDate: enoughPastTime(),
})),
];

const output = aggregate(events, t + 300);

expect(output).toStrictEqual({
activities: [
activity({
id: "A",
name: "sample",
transitionState: "enter-done",
params: { newParam: "value" },
steps: [
{
id: "s1",
params: { newParam: "value" },
enteredBy: stepReplacedEvent,
zIndex: 0,
hasZIndex: false,
},
],
enteredBy: pushedEvent1,
isActive: false,
isTop: false,
isRoot: true,
zIndex: 0,
}),
activity({
id: "B",
name: "sample",
transitionState: "enter-done",
params: {},
steps: [
{
id: "B",
params: {},
enteredBy: pushedEvent2,
zIndex: 1,
},
],
enteredBy: pushedEvent2,
isActive: true,
isTop: true,
isRoot: false,
zIndex: 1,
}),
],
registeredActivities: [
{
name: "sample",
},
],
transitionDuration: 300,
globalTransitionState: "idle",
events,
});
});

Comment thread
coderabbitai[bot] marked this conversation as resolved.
test("aggregate - StepPoppedEvent can target lower activity with targetActivityId", () => {
const t = nowTime();

let pushedEvent1: PushedEvent;
let pushedEvent2: PushedEvent;
let stepPushedEvent1: StepPushedEvent;
let stepPushedEvent2: StepPushedEvent;
let stepPoppedEvent: StepPoppedEvent;

const events = [
initializedEvent({
transitionDuration: 300,
}),
registeredEvent({
activityName: "sample",
}),
(pushedEvent1 = makeEvent("Pushed", {
activityId: "A",
activityName: "sample",
activityParams: {},
eventDate: enoughPastTime(),
})),
(stepPushedEvent1 = makeEvent("StepPushed", {
stepId: "s1",
stepParams: { step: "1" },
eventDate: enoughPastTime(),
})),
(stepPushedEvent2 = makeEvent("StepPushed", {
stepId: "s2",
stepParams: { step: "2" },
eventDate: enoughPastTime(),
})),
(pushedEvent2 = makeEvent("Pushed", {
activityId: "B",
activityName: "sample",
activityParams: {},
eventDate: enoughPastTime(),
})),
(stepPoppedEvent = makeEvent("StepPopped", {
targetActivityId: pushedEvent1.activityId,
eventDate: enoughPastTime(),
})),
];

const output = aggregate(events, t + 300);

expect(output).toStrictEqual({
activities: [
activity({
id: "A",
name: "sample",
transitionState: "enter-done",
params: { step: "1" },
steps: [
{
id: "A",
params: {},
enteredBy: pushedEvent1,
zIndex: 0,
},
{
id: "s1",
params: { step: "1" },
enteredBy: stepPushedEvent1,
zIndex: 0,
hasZIndex: false,
},
],
enteredBy: pushedEvent1,
isActive: false,
isTop: false,
isRoot: true,
zIndex: 0,
}),
activity({
id: "B",
name: "sample",
transitionState: "enter-done",
params: {},
steps: [
{
id: "B",
params: {},
enteredBy: pushedEvent2,
zIndex: 1,
},
],
enteredBy: pushedEvent2,
isActive: true,
isTop: true,
isRoot: false,
zIndex: 1,
}),
],
registeredActivities: [
{
name: "sample",
},
],
transitionDuration: 300,
globalTransitionState: "idle",
events,
});
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
51 changes: 51 additions & 0 deletions extensions/plugin-history-sync/src/historySyncPlugin.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1463,4 +1463,55 @@ describe("historySyncPlugin", () => {
*/
expect(queryResponse.data.hello).toEqual("world");
});

test("historySyncPlugin - stepPop on lower activity skips removed step when navigating back", async () => {
actions.push({
activityId: "a1",
activityName: "Article",
activityParams: {
articleId: "10",
title: "step1",
},
});

actions.stepPush({
stepId: "s1",
stepParams: {
articleId: "11",
title: "step2",
},
});

actions.stepPush({
stepId: "s2",
stepParams: {
articleId: "12",
title: "step3",
},
});

await actions.push({
activityId: "a2",
activityName: "Article",
activityParams: {
articleId: "20",
title: "second",
},
});

expect(history.index).toEqual(4);

// Pop step from lower activity a1
actions.stepPop({ targetActivityId: "a1" });

// Navigate back - should skip the removed step3 entry and land on step2
history.back();

const stack = await actions.getStack();
const active = activeActivity(stack);
expect(active?.id).toEqual("a1");
expect(active?.steps.length).toEqual(2); // step1 and step2 remain
expect(active?.steps[1]?.params.title).toEqual("step2");
expect(path(history.location)).toEqual("/articles/11/?title=step2");
});
Comment on lines +1467 to +1516
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Add test coverage for multiple step pops on lower activity.

The test correctly verifies single step pop behavior, but doesn't cover the scenario where multiple steps are popped from a lower activity. Given the past review comment flagging a critical issue with multi-step pops (only calling history.back() once instead of history.go(-delta)), add a test case that:

  1. Pops multiple steps from a lower activity (e.g., call stepPop twice with targetActivityId)
  2. Navigates back with history.back()
  3. Verifies that all removed steps are skipped, not just one

This would catch the bug in the implementation where only one step is skipped.

Example test structure:

test("historySyncPlugin - multiple stepPops on lower activity skip all removed steps when navigating back", async () => {
  // Setup: activity a1 with 3 steps, then activity a2
  actions.push({ activityId: "a1", activityName: "Article", activityParams: { articleId: "10", title: "step1" } });
  actions.stepPush({ stepId: "s1", stepParams: { articleId: "11", title: "step2" } });
  actions.stepPush({ stepId: "s2", stepParams: { articleId: "12", title: "step3" } });
  actions.stepPush({ stepId: "s3", stepParams: { articleId: "13", title: "step4" } });
  await actions.push({ activityId: "a2", activityName: "Article", activityParams: { articleId: "20", title: "second" } });
  
  // Pop TWO steps from lower activity a1
  actions.stepPop({ targetActivityId: "a1" });
  actions.stepPop({ targetActivityId: "a1" });
  
  // Navigate back - should skip both removed steps (step4 and step3) and land on step2
  history.back();
  
  const stack = await actions.getStack();
  const active = activeActivity(stack);
  expect(active?.id).toEqual("a1");
  expect(active?.steps.length).toEqual(2);
  expect(active?.steps[1]?.params.title).toEqual("step2");
  expect(path(history.location)).toEqual("/articles/11/?title=step2");
});
🤖 Prompt for AI Agents
In extensions/plugin-history-sync/src/historySyncPlugin.spec.ts around lines
1467 to 1516, add a new test that covers popping multiple steps from a lower
activity and ensures history navigation skips all removed steps (not just one);
specifically, create activity "a1" with at least 4 steps, push a second activity
"a2", call actions.stepPop({ targetActivityId: "a1" }) twice to remove two lower
steps, call history.back(), then assert the active activity is "a1", that its
steps length reflects the two remaining steps, that the last step's title
matches the expected remaining step (e.g., "step2"), and that
history.location.path matches the URL for that step; this mirrors the existing
single-pop test but repeats the stepPop call to validate multi-pop behavior so
tests fail if code still only calls history.back() once instead of
history.go(-delta).

});
Loading
Loading