Skip to content
Open
Show file tree
Hide file tree
Changes from 7 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
29 changes: 29 additions & 0 deletions .changeset/lower-activity-step-actions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
---
"@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 previous activity parameters when popping current activity

**History Sync Plugin Changes:**
- Added intelligent synchronization when navigating to modified lower activities
- Uses `history.back()` for step pop to avoid duplicate entries
- Uses `replaceState()` for step push/replace (accepts forward history volatility per stack semantics)
- Handles complex step operation sequences correctly

**Use Case:**
```typescript
// Pop and modify previous activity in single operation
actions.pop({
onBeforePop: () => {
actions.stepReplace({ newParams }, { targetActivityId: previousActivityId });
}
});
```

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.
54 changes: 54 additions & 0 deletions extensions/plugin-history-sync/src/historySyncPlugin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,60 @@ export function historySyncPlugin<
(step) => step.id === targetStep?.id,
);

// Synchronize history state with core state for lower activity step modifications
// Only apply this logic for non-active (lower) activities to avoid interfering
// with normal step navigation on the active activity
if (nextActivity && !nextActivity.isActive) {
const coreSteps = nextActivity.steps;
const coreLastStep = last(coreSteps);
const historyStep = targetStep;
const historySteps = targetActivity.steps;

// Check if history step exists in core state
const historyStepExistsInCore = historyStep
? coreSteps.some((step) => step.id === historyStep.id)
: true;

if (!historyStepExistsInCore) {
// History step was removed or replaced
if (coreSteps.length < historySteps.length) {
// Step Pop: Step count decreased
// Use history.back() to skip the removed step entry
requestHistoryTick(() => {
silentFlag = true;
history.back();
});
return;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Multi-step pop only calls back() once — can leave user on an invalid, still-removed step.

If core popped N>1 steps on a lower activity, a single history.back() won’t skip all removed entries. Use history.go(-delta) to jump exactly the difference.

Apply this diff:

-              if (coreSteps.length < historySteps.length) {
-                // Step Pop: Step count decreased
-                // Use history.back() to skip the removed step entry
-                requestHistoryTick(() => {
-                  silentFlag = true;
-                  history.back();
-                });
-                return;
-              } else {
+              if (coreSteps.length < historySteps.length) {
+                // Step Pop: jump back by the exact number of removed steps
+                const popDelta = historySteps.length - coreSteps.length;
+                if (popDelta > 0) {
+                  requestHistoryTick(() => {
+                    silentFlag = true;
+                    history.go(-popDelta);
+                  });
+                }
+                return;
+              } else {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (coreSteps.length < historySteps.length) {
// Step Pop: Step count decreased
// Use history.back() to skip the removed step entry
requestHistoryTick(() => {
silentFlag = true;
history.back();
});
return;
if (coreSteps.length < historySteps.length) {
// Step Pop: jump back by the exact number of removed steps
const popDelta = historySteps.length - coreSteps.length;
if (popDelta > 0) {
requestHistoryTick(() => {
silentFlag = true;
history.go(-popDelta);
});
}
return;
🤖 Prompt for AI Agents
In extensions/plugin-history-sync/src/historySyncPlugin.tsx around lines 414 to
421, the code only calls history.back() when coreSteps.length <
historySteps.length which only moves back one entry; compute the exact delta =
historySteps.length - coreSteps.length and inside the requestHistoryTick
callback set silentFlag = true and call history.go(-delta) so the browser jumps
back by the full number of removed entries (then return as before).

} else {
// Step Replace or complex operations: Step count same or increased
// Use replaceState to show current core state
const match = activityRoutes.find(
(r) => r.activityName === nextActivity.name,
)!;
const template = makeTemplate(match, options.urlPatternOptions);

requestHistoryTick(() => {
silentFlag = true;
replaceState({
history,
pathname: template.fill(nextActivity.params),
state: {
activity: nextActivity,
step: coreLastStep,
},
useHash: options.useHash,
});
});
return;
}
}

// If history step exists in core, proceed with normal navigation logic
// This handles:
// - Step exists but is not the last (normal step backward)
// - Multiple step pushes then navigating to middle steps
}

const isBackward = () => currentActivity.id > targetActivityId;
const isForward = () => currentActivity.id < targetActivityId;
const isStep = () => currentActivity.id === targetActivityId;
Expand Down
Loading