Skip to content

Commit 8807c12

Browse files
committed
WIP: attempt stepPush/stepReplace sync - has regression
**Attempted:** Implemented onPopState synchronization for stepPush and stepReplace on lower activities to sync history when navigating back. **Implementation:** - Case 1: History has step that doesn't exist in core → sync - Case 2: History has no step but core has steps → sync **Problem:** Case 2 condition `!targetStep && coreSteps.length > 0` is too broad and triggers for normal activity navigation, causing regression. **Failed Tests:** - ✗ "히스토리를 여러번 back하더라도" (regression) - ✗ "stepPush on lower activity syncs when navigating back" (new test) **Root Cause:** Cannot distinguish between: 1. [A] [B] → stepPush to A → [A(step)] [B] → back (should sync) 2. [A] [B(step)] → back to A (normal, should not sync) Both cases have `!targetStep` when arriving at A. **Options to Consider:** 1. Revert to stepPop-only (safe but incomplete) 2. Add "modified" marker to history state (complex) 3. Only handle step count increase: `coreSteps.length > historySteps.length` (safer but doesn't handle stepReplace) **Status:** 2 tests failing, 28 passing. Need decision on approach. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent d951a66 commit 8807c12

2 files changed

Lines changed: 78 additions & 14 deletions

File tree

extensions/plugin-history-sync/src/historySyncPlugin.spec.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1558,4 +1558,44 @@ describe("historySyncPlugin", () => {
15581558
expect(lowerActivity?.steps.length).toEqual(2); // initial params + added step
15591559
expect(lowerActivity?.steps[1]?.params.title).toEqual("added-step");
15601560
});
1561+
1562+
test("historySyncPlugin - stepPush on lower activity syncs when navigating back", async () => {
1563+
actions.push({
1564+
activityId: "a1",
1565+
activityName: "Article",
1566+
activityParams: {
1567+
articleId: "10",
1568+
title: "first",
1569+
},
1570+
});
1571+
1572+
await actions.push({
1573+
activityId: "a2",
1574+
activityName: "Article",
1575+
activityParams: {
1576+
articleId: "20",
1577+
title: "second",
1578+
},
1579+
});
1580+
1581+
// Add step to lower activity a1 while at a2
1582+
await actions.stepPush({
1583+
stepId: "s1",
1584+
stepParams: {
1585+
articleId: "15",
1586+
title: "added-step",
1587+
},
1588+
targetActivityId: "a1",
1589+
});
1590+
1591+
// Navigate back to a1 - should sync to show the added step
1592+
history.back();
1593+
1594+
const stack = await actions.getStack();
1595+
const active = activeActivity(stack);
1596+
expect(active?.id).toEqual("a1");
1597+
expect(active?.steps.length).toEqual(2);
1598+
expect(active?.steps[1]?.params.title).toEqual("added-step");
1599+
expect(path(history.location)).toEqual("/articles/15/?title=added-step");
1600+
});
15611601
});

extensions/plugin-history-sync/src/historySyncPlugin.tsx

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -399,24 +399,48 @@ export function historySyncPlugin<
399399
// Synchronize history state with core state for lower activity step modifications
400400
// Only apply this logic for non-active (lower) activities to avoid interfering
401401
// with normal step navigation on the active activity
402-
if (nextActivity && !nextActivity.isActive && targetStep) {
402+
if (nextActivity && !nextActivity.isActive) {
403403
const coreSteps = nextActivity.steps;
404+
const coreLastStep = last(coreSteps);
404405
const historySteps = targetActivity.steps;
405406

406-
// Check if we're navigating to a step that was removed (stepPop scenario)
407-
const historyStepExistsInCore = coreSteps.some(
408-
(step) => step.id === targetStep.id,
409-
);
407+
let needsSync = false;
410408

411-
if (
412-
!historyStepExistsInCore &&
413-
coreSteps.length < historySteps.length
414-
) {
415-
// Step Pop: The step we're navigating to was removed
416-
// Use history.back() to skip the removed step entry
417-
// Don't set silentFlag - we want the next popstate to update app state
418-
history.back();
419-
return;
409+
// Case 1: History has a step that doesn't exist in core (stepPop or stepReplace)
410+
if (targetStep && !coreSteps.some((step) => step.id === targetStep?.id)) {
411+
if (coreSteps.length < historySteps.length) {
412+
// Step Pop: Use history.back() to skip removed step entry
413+
history.back();
414+
return;
415+
}
416+
needsSync = true; // stepReplace or complex operations
417+
}
418+
// Case 2: History has no step but core has steps (stepPush added to lower activity)
419+
else if (!targetStep && coreSteps.length > 0) {
420+
needsSync = true;
421+
}
422+
423+
if (needsSync) {
424+
// Sync history with core state
425+
const match = activityRoutes.find(
426+
(r) => r.activityName === nextActivity.name,
427+
)!;
428+
const template = makeTemplate(match, options.urlPatternOptions);
429+
const paramsForUrl = coreLastStep?.params ?? nextActivity.params;
430+
431+
replaceState({
432+
history,
433+
pathname: template.fill(paramsForUrl),
434+
state: {
435+
activity: nextActivity,
436+
step: coreLastStep,
437+
},
438+
useHash: options.useHash,
439+
});
440+
441+
// Update target variables for normal navigation logic
442+
targetActivity = nextActivity;
443+
targetStep = coreLastStep;
420444
}
421445
}
422446

0 commit comments

Comments
 (0)