diff --git a/.changeset/coerce-activity-params-to-string.md b/.changeset/coerce-activity-params-to-string.md new file mode 100644 index 000000000..cb463d1ea --- /dev/null +++ b/.changeset/coerce-activity-params-to-string.md @@ -0,0 +1,15 @@ +--- +"@stackflow/plugin-history-sync": minor +--- + +Coerce activity/step params to `string | undefined` at the plugin boundary. + +Before this change, `push("X", { visible: true })` would store the boolean `true` in the core store while URL-arrival parsed the same URL as `{ visible: "true" }`, so `useActivityParams()` returned different runtime types depending on how the user reached the activity. This PR coerces non-string values to strings inside `plugin-history-sync`'s `onBeforePush` / `onBeforeReplace` / `onBeforeStepPush` / `onBeforeStepReplace` hooks (after `encode` consumes the typed params to build the URL), and on the `decode`-path in `overrideInitialEvents`, so the core store always contains `{ [key: string]: string | undefined }`. `encode` still receives the typed params `U` from `template.fill`. Post-effect hooks (`onPushed`, `onReplaced`, `onStepPushed`, `onStepReplaced`, `onInit`) now use the new `fillWithoutEncode` to avoid re-running `encode` on already-coerced store values. + +This is a behavioral change for consumers that relied on internal push preserving non-string values in the store (a pre-existing divergence from URL-arrival behavior). See the docs update for the migration note. + +Migration notes: + +- If you authored a `decode` hook that returns typed values (e.g. `decode: (p) => ({ count: Number(p.count) })`), those return values are now coerced back to strings in the store to match the declared `ActivityBaseParams` contract. Move runtime type coercion to the usage site (`Number(useActivityParams().count)`). +- If your app registers a plugin AFTER `historySyncPlugin` in the plugins array and that plugin re-injects typed values via `overrideActionParams`, those values will NOT be coerced by this plugin. Register `historySyncPlugin` last among plugins that mutate `activityParams` to preserve the string-only invariant. +- Cross-deploy hydration: when a user reloads on a deploy that includes this fix after a previous deploy serialized typed values into `history.state`, the params are coerced to strings at hydration time inside the `parseState` early-return. No consumer change required — the post-fix runtime contract (`useActivityParams()` returns `string | undefined`) holds across version boundaries. diff --git a/.changeset/step-context-path.md b/.changeset/step-context-path.md new file mode 100644 index 000000000..d3da1577d --- /dev/null +++ b/.changeset/step-context-path.md @@ -0,0 +1,12 @@ +--- +"@stackflow/core": minor +"@stackflow/plugin-history-sync": patch +--- + +Add optional `stepContext.path?: string` to `StepPushedEvent` and `StepReplacedEvent` (purely additive, no breaking change). `@stackflow/plugin-history-sync` uses this to preserve `encode`-output URLs through the store across every step navigation path — including `popstate` forward across step boundaries — instead of relying on plugin-internal state. + +This addresses three regressions surfaced in PR review: + +1. **`encode` output not in `history.location`** — post-effect hooks (`onPushed` / `onReplaced` / `onStepPushed` / `onStepReplaced` / `onInit`) called `template.fillWithoutEncode(activity.params)` against the post-coercion strings, skipping `encode` and writing coerced values into the URL. Now they read the encoded URL pre-computed in pre-effect hooks (`activityContext.path` / `stepContext.path`), with `fillWithoutEncode` as a defensive fallback only. +2. **`encode` called with coerced strings on popstate forward re-push** — the popstate `isForward` and `isStepForward` branches reconstructed push events without preserving `activityContext` / `stepContext`, causing `onBeforePush` / `onBeforeStepPush` to call `template.fill` with already-coerced strings. Now those branches pass `activityContext: targetActivity.context` / `stepContext: targetStep.context`, and the pre-effect hooks short-circuit when the path is already present (`"path" in actionParams.activityContext`). +3. **Test gap: `path(history.location)` was never asserted under non-identity `encode`** — every existing test asserted `activity.context.path` only. Added 15 new tests asserting the URL surface under non-identity encode, including popstate-forward across activity AND step boundaries, `defaultHistory` ancestor URLs, SSR replay, and `replace`-with-active-steps. diff --git a/core/src/Stack.ts b/core/src/Stack.ts index fe2c0dfc2..4c116a394 100644 --- a/core/src/Stack.ts +++ b/core/src/Stack.ts @@ -1,4 +1,3 @@ -import type { BaseDomainEvent } from "event-types/_base"; import type { DomainEvent, PoppedEvent, @@ -19,6 +18,7 @@ export type ActivityStep = { params: { [key: string]: string | undefined; }; + context?: {}; enteredBy: PushedEvent | ReplacedEvent | StepPushedEvent | StepReplacedEvent; exitedBy?: ReplacedEvent | PoppedEvent | StepReplacedEvent | StepPoppedEvent; zIndex: number; diff --git a/core/src/activity-utils/makeActivityReducer.ts b/core/src/activity-utils/makeActivityReducer.ts index fac85e2d7..616f0c38a 100644 --- a/core/src/activity-utils/makeActivityReducer.ts +++ b/core/src/activity-utils/makeActivityReducer.ts @@ -68,6 +68,7 @@ export function makeActivityReducer(context: { const newRoute = { id: event.stepId, params: event.stepParams, + ...(event.stepContext ? { context: event.stepContext } : null), enteredBy: event, zIndex: activity.zIndex, hasZIndex: event.hasZIndex ?? false, @@ -88,6 +89,7 @@ export function makeActivityReducer(context: { const newRoute = { id: event.stepId, params: event.stepParams, + ...(event.stepContext ? { context: event.stepContext } : null), enteredBy: event, zIndex: activity.zIndex, hasZIndex: event.hasZIndex ?? false, @@ -107,7 +109,7 @@ export function makeActivityReducer(context: { * Pop the last step * If there are params in the previous step, set them as the new params */ - StepPopped: (activity: Activity, event: StepPoppedEvent): Activity => { + StepPopped: (activity: Activity, _event: StepPoppedEvent): Activity => { activity.steps.pop(); const beforeActivityParams = last(activity.steps)?.params; diff --git a/core/src/aggregate.ts b/core/src/aggregate.ts index d8db38d98..83284c935 100644 --- a/core/src/aggregate.ts +++ b/core/src/aggregate.ts @@ -75,6 +75,7 @@ export function aggregate(inputEvents: DomainEvent[], now: number): Stack { { ...step, zIndex: lastStepZIndex + (step.hasZIndex ? 1 : 0), + ...(step.context ? { context: step.context } : null), }, ]; }, []); diff --git a/core/src/event-types/StepPushedEvent.ts b/core/src/event-types/StepPushedEvent.ts index f962b4766..2ac5f9699 100644 --- a/core/src/event-types/StepPushedEvent.ts +++ b/core/src/event-types/StepPushedEvent.ts @@ -7,6 +7,7 @@ export type StepPushedEvent = BaseDomainEvent< stepParams: { [key: string]: string | undefined; }; + stepContext?: {}; targetActivityId?: string; hasZIndex?: boolean; } diff --git a/core/src/event-types/StepReplacedEvent.ts b/core/src/event-types/StepReplacedEvent.ts index 39975e4c0..b2231dcaf 100644 --- a/core/src/event-types/StepReplacedEvent.ts +++ b/core/src/event-types/StepReplacedEvent.ts @@ -7,6 +7,7 @@ export type StepReplacedEvent = BaseDomainEvent< stepParams: { [key: string]: string | undefined; }; + stepContext?: {}; targetActivityId?: string; hasZIndex?: boolean; } diff --git a/docs/pages/api-references/future-api/changes.en.mdx b/docs/pages/api-references/future-api/changes.en.mdx index ef7a253db..fbde0ff64 100644 --- a/docs/pages/api-references/future-api/changes.en.mdx +++ b/docs/pages/api-references/future-api/changes.en.mdx @@ -126,6 +126,22 @@ declare module "@stackflow/config" { } ``` +### Runtime Coercion of Activity Params (FEP-1061) + +Regardless of how an activity is entered — `push()`, `replace()`, `stepPush()`, `stepReplace()`, or URL arrival (with or without a `decode` hook) — the params you receive from `useActivityParams()` are always `string | undefined` at runtime. + +```tsx +// These two paths used to produce different runtime types. They don't anymore. +push("Article", { visible: true }) // store: { visible: "true" } +// URL arrival: /articles/1?visible=true // store: { visible: "true" } +``` + +The `encode` hook on a route still receives the original typed params `U` (so you can use `encode: ({ visible }) => ({ visible: visible ? "y" : "n" })` exactly as before). Coercion happens at the `@stackflow/plugin-history-sync` boundary, *after* `encode` has consumed the typed values to build the URL. + + + **Migration note for `decode` users**: if you previously relied on `decode` to inject typed values (e.g. `decode: (p) => ({ count: Number(p.count) })`) and read them back via `useActivityParams().count` as a number, that value is now a string in the store. Perform the type coercion at the usage site instead: `Number(params.count)`. + + ## `useFlow()`, `useStepFlow()` You no longer need to create hooks like `useFlow()` and `useStepFlow()` using functions like `flow()`. You can import them directly. diff --git a/docs/pages/api-references/future-api/changes.ko.mdx b/docs/pages/api-references/future-api/changes.ko.mdx index 9d577b9c6..089ef0245 100644 --- a/docs/pages/api-references/future-api/changes.ko.mdx +++ b/docs/pages/api-references/future-api/changes.ko.mdx @@ -126,6 +126,22 @@ declare module "@stackflow/config" { } ``` +### 액티비티 파라미터의 런타임 강제 변환 (FEP-1061) + +액티비티에 어떻게 진입했는지에 관계없이 — `push()`, `replace()`, `stepPush()`, `stepReplace()`, 또는 URL로의 직접 진입(`decode` 유무와 무관) — `useActivityParams()`로 받는 파라미터는 런타임에 항상 `string | undefined` 예요. + +```tsx +// 이전에는 두 경로가 런타임에 서로 다른 타입을 반환했지만, 이제는 동일해요. +push("Article", { visible: true }) // 스토어: { visible: "true" } +// URL 진입: /articles/1?visible=true // 스토어: { visible: "true" } +``` + +라우트의 `encode` 훅은 여전히 원본의 타입이 적용된 파라미터 `U`를 받아요 (예: `encode: ({ visible }) => ({ visible: visible ? "y" : "n" })`는 그대로 동작해요). 문자열화는 `encode`가 URL 생성을 위해 타입이 적용된 값을 소비한 *이후에*, `@stackflow/plugin-history-sync` 경계에서 이뤄져요. + + + **`decode` 사용자를 위한 마이그레이션 안내**: 이전에 `decode`로 타입이 적용된 값을 주입해서 (예: `decode: (p) => ({ count: Number(p.count) })`) `useActivityParams().count`를 숫자로 사용하셨다면, 이제 해당 값은 스토어에서 문자열이에요. 사용 지점에서 타입 변환을 해주세요: `Number(params.count)`. + + ## `useFlow()`, `useStepFlow()` 이제 `flow()` 등의 함수로 `useFlow()`, `useStepFlow()` 등의 훅을 생성할 필요가 없어요. 바로 import해서 쓸 수 있어요. diff --git a/extensions/plugin-history-sync/INTENT.md b/extensions/plugin-history-sync/INTENT.md new file mode 100644 index 000000000..d8b7fc55e --- /dev/null +++ b/extensions/plugin-history-sync/INTENT.md @@ -0,0 +1,71 @@ +# Activity params runtime contract — design intent + +This document captures the **chosen interpretation** and **boundary decision** for activity / step params runtime types in `@stackflow/plugin-history-sync`. It exists to record why the runtime contract was tightened to "always string" rather than widened to allow arbitrary types. + +## Chosen interpretation + +**Always-string at the plugin boundary.** `useActivityParams()` returns `Record` regardless of how an activity is entered — `push` / `replace` / `stepPush` / `stepReplace` / URL arrival with or without `decode` / cross-deploy `historyState` hydration. + +The originating user request was not "let me pass typed values into the store" but rather "stop the auto-typecast that makes `useActivityParams` return a different runtime type depending on entry path". Internal navigation (`push({ visible: true })`) used to leave a boolean in the store while URL-arrival parsed the same value as `"true"`. Consumers were string-converting params before every push as a workaround. + +This implementation does **not** widen `ActivityBaseParams = { [key: string]?: string }`; it enforces that declaration at runtime so the type and the runtime stop diverging. + +## Boundary decision + +**Coercion lives at the `@stackflow/plugin-history-sync` boundary, not in `@stackflow/core`.** + +Concretely, `coerceParamsToString` runs inside the plugin's pre-effect hooks (`onBeforePush`, `onBeforeReplace`, `onBeforeStepPush`, `onBeforeStepReplace`) and inside `overrideInitialEvents` for the URL-arrival and cross-deploy hydration paths. `@stackflow/core` itself does not coerce. + +### Tradeoff + +A consumer that uses `@stackflow/core` *without* `historySyncPlugin` (e.g., programmatic-only navigation with no URL sync) does NOT receive the coercion. That consumer's store will contain whatever typed values they passed to `push()`. This is a documented tradeoff: + +- **Pro:** keeps `@stackflow/core` framework-agnostic and free of plugin-specific concerns. +- **Pro:** consumers who don't need URL sync don't pay the coercion cost. +- **Con:** the invariant is plugin-conditional, not core-universal. Consumers swapping `historySyncPlugin` for a different sync plugin must implement equivalent coercion. + +### Why this tradeoff was chosen + +1. `historySyncPlugin` is the only first-party plugin that serializes params to a string-shaped destination (URL). Without that destination, string-coercion has no architectural justification. +2. Moving coercion into `@stackflow/core` would make the core opinionated about a serialization concern that's strictly a plugin's responsibility. +3. The `ActivityBaseParams` type declaration (`{ [key: string]?: string }`) already pins consumer expectations at compile time; runtime coercion at the plugin boundary brings the runtime into alignment with the declared type, but the type itself remains the source of truth for non-history-sync consumers. + +If a future requirement establishes that the invariant should be a core-store contract instead, the implementation has to move into `@stackflow/core`'s reducer and the `coerceParamsToString` utility migrates with it. + +## Plugin order matters (documented limitation) + +If a plugin registered AFTER `historySyncPlugin` in the plugins array calls `overrideActionParams` with typed values, those values bypass `historySyncPlugin`'s pre-effect coercion and land in the store as-is. This is locked as a regression test so it cannot silently regress. + +**Consumer guidance:** register `historySyncPlugin` last among plugins that mutate `activityParams`. The changeset for this work documents this. + +## Cross-deploy hydration + +`overrideInitialEvents`'s `parseState` early-return deserializes activity / step state previously written to `history.state`. If an old deploy wrote typed values, the new deploy's `coerceParamsToString` calls coerce them at hydration time. Idempotent on already-coerced strings. + +## URL output contract — `history.location` reflects `encode` output + +The runtime contract for `useActivityParams()` is "always string". The contract for `history.location` is **independent**: it reflects `encode` output for routes with a custom `encode`. + +To uphold both contracts: + +1. Pre-effect hooks (`onBeforePush` / `onBeforeReplace` / `onBeforeStepPush` / `onBeforeStepReplace`) compute the encoded URL via `template.fill(typed_params)` BEFORE coercion. Activities store this in `activityContext.path` (already in core); steps store it in `stepContext.path`. +2. Post-effect hooks (`onPushed` / `onReplaced` / `onStepPushed` / `onStepReplaced` / `onInit`) read `activity.context.path` and `step.context.path` directly. They never re-run `encode` on coerced strings. +3. The popstate `isForward` and `isStepForward` branches preserve `activityContext` / `stepContext` from the stored target, so the encoded URL is recovered without re-running `encode`. +4. If `*.context.path` is missing (e.g. a third-party plugin dispatched a `Pushed` event without going through `onBeforePush`, or a pre-update `history.state` was hydrated from an older deploy), post-effect hooks fall back to `template.fillWithoutEncode(coerced_params)` — same lossy behavior as before this change, but only on those bypass paths. + +### SSR consideration + +When the server emits `activity.context.path` (e.g. via `initialContext.req.path` flowing through `historyEntryToEvents`), the client's `onInit` URL-replay trusts the server-emitted path rather than recomputing. This avoids encode-version mismatches between server and client builds. If you upgrade `encode` for a route, redeploy server and client together. + +### Cross-deploy hydration legacy fallback + +Entries serialized into `history.state` before `stepContext.path` landed on core events have no `step.context.path`. On URL-arrival into such state, `onBeforeStepPush` runs the recompute branch — which requires the parent activity to be present in the stack. During initial boot, the parent might not be materialized yet, so recompute is skipped and post-effect falls back to `fillWithoutEncode(coerced)`. Acceptable as a transitional state across one deploy boundary; subsequent navigations populate `stepContext.path` correctly. + +## Decision record + +- **Decision:** runtime contract is "always-string at the plugin boundary"; the underlying type declaration is unchanged. +- **Drivers:** the originating user complaint was about runtime type divergence between in-process navigation and URL arrival, not about wanting typed values in the store. Type-widening would force every consumer to handle non-string runtime types at the usage site. +- **Alternatives considered:** (a) widen `ActivityBaseParams` to `unknown`; (b) move coercion into `@stackflow/core`. Both rejected — see "Boundary decision" tradeoff above. +- **Why chosen:** keeps the type contract stable, fixes the runtime divergence at the plugin layer that owns the URL-serialization concern. +- **Consequences:** programmatic-only consumers (no `historySyncPlugin`) keep typed values in store; the invariant is plugin-conditional. Documented for future maintainers. +- **Follow-ups:** if a future requirement prefers direction (a) or (b), a new ticket should track that work; this implementation does not pre-empt it. diff --git a/extensions/plugin-history-sync/src/coerceParamsToString.spec.ts b/extensions/plugin-history-sync/src/coerceParamsToString.spec.ts new file mode 100644 index 000000000..cd3c7063d --- /dev/null +++ b/extensions/plugin-history-sync/src/coerceParamsToString.spec.ts @@ -0,0 +1,254 @@ +import { coerceParamsToString } from "./coerceParamsToString"; + +test("coerceParamsToString - returns empty object when params is null", () => { + expect(coerceParamsToString(null)).toStrictEqual({}); +}); + +test("coerceParamsToString - returns empty object when params is undefined", () => { + expect(coerceParamsToString(undefined)).toStrictEqual({}); +}); + +test("coerceParamsToString - keeps string values unchanged", () => { + expect(coerceParamsToString({ name: "hello", empty: "" })).toStrictEqual({ + name: "hello", + empty: "", + }); +}); + +test("coerceParamsToString - coerces booleans to strings", () => { + expect(coerceParamsToString({ visible: true, hidden: false })).toStrictEqual({ + visible: "true", + hidden: "false", + }); +}); + +test("coerceParamsToString - coerces numbers to strings (including zero)", () => { + expect(coerceParamsToString({ count: 5, zero: 0, neg: -1 })).toStrictEqual({ + count: "5", + zero: "0", + neg: "-1", + }); +}); + +test("coerceParamsToString - coerces bigint to string", () => { + expect(coerceParamsToString({ big: BigInt(10) })).toStrictEqual({ + big: "10", + }); +}); + +test("coerceParamsToString - coerces symbol to string", () => { + const sym = Symbol("foo"); + expect(coerceParamsToString({ sym })).toStrictEqual({ + sym: String(sym), + }); +}); + +test("coerceParamsToString - preserves undefined values", () => { + expect(coerceParamsToString({ opt: undefined })).toStrictEqual({ + opt: undefined, + }); +}); + +test("coerceParamsToString - converts null values to undefined", () => { + expect(coerceParamsToString({ opt: null })).toStrictEqual({ + opt: undefined, + }); +}); + +test("coerceParamsToString - stringifies plain objects via JSON.stringify", () => { + expect( + coerceParamsToString({ filter: { category: "tech", count: 3 } }), + ).toStrictEqual({ + filter: '{"category":"tech","count":3}', + }); +}); + +test("coerceParamsToString - stringifies arrays via JSON.stringify", () => { + expect(coerceParamsToString({ tags: ["js", "ts"] })).toStrictEqual({ + tags: '["js","ts"]', + }); +}); + +test("coerceParamsToString - handles circular references without throwing", () => { + const circular: Record = {}; + circular.self = circular; + const result = coerceParamsToString({ circular }); + // When JSON.stringify fails, fall back to String() + expect(typeof result.circular).toBe("string"); + expect(result.circular).toBe(String(circular)); +}); + +test("coerceParamsToString - coerces functions", () => { + const fn = () => "hello"; + const result = coerceParamsToString({ fn }); + expect(typeof result.fn).toBe("string"); +}); + +test("coerceParamsToString - handles nested objects with undefined values", () => { + expect( + coerceParamsToString({ outer: { inner: undefined, value: 1 } }), + ).toStrictEqual({ + outer: '{"value":1}', + }); +}); + +test("coerceParamsToString - handles mixed types", () => { + expect( + coerceParamsToString({ + a: true, + b: 5, + c: "x", + d: undefined, + e: null, + }), + ).toStrictEqual({ + a: "true", + b: "5", + c: "x", + d: undefined, + e: undefined, + }); +}); + +test("coerceParamsToString - is idempotent for all covered input types", () => { + const sym = Symbol("foo"); + const fn = () => "hello"; + const circular: Record = {}; + circular.self = circular; + + const input = { + str: "x", + emptyStr: "", + boolT: true, + boolF: false, + num: 5, + zero: 0, + neg: -1, + big: BigInt(10), + sym, + undef: undefined, + nullVal: null, + obj: { a: 1, b: [1, 2] }, + arr: ["js", "ts"], + fn, + circular, + }; + + const once = coerceParamsToString(input); + const twice = coerceParamsToString(once); + + expect(twice).toStrictEqual(once); + + // T-U-NEW-9: post-coerce typeof for every key must be "string" or + // "undefined". Deep-equality alone permits a regression where one branch + // produces a non-string (e.g. a number) AND idempotently maps to itself. + // This loop closes that gap: the post-condition of `coerceParamsToString` + // is that every emitted value satisfies the `string | undefined` contract, + // not just that running coerce twice produces the same shape. + for (const k of Object.keys(once)) { + const v = (once as Record)[k]; + expect(typeof v === "string" || typeof v === "undefined").toBe(true); + } +}); + +test("coerceParamsToString - circular ref forces String() fallback (deterministic, T-U-NEW-8)", () => { + // T-U-NEW-8: the original circular-ref test relied on `JSON.stringify` + // genuinely throwing on a self-referential object (which it does in V8), + // but that behavior is engine-dependent and the test's coverage of the + // catch branch is implicit. This deterministically forces the catch path + // by stubbing `JSON.stringify` to throw, then asserts the `String(value)` + // fallback was actually taken on a normal object input. + const spy = jest.spyOn(JSON, "stringify").mockImplementationOnce(() => { + throw new Error("forced"); + }); + try { + const value = { a: 1 }; + const result = coerceParamsToString({ obj: value }); + expect(result.obj).toBe(String(value)); // "[object Object]" + expect(typeof result.obj).toBe("string"); + expect(spy).toHaveBeenCalled(); + } finally { + spy.mockRestore(); + } +}); + +test("coerceParamsToString - handles a 1000-key record with mixed types: spot-checks every cycle branch with typeof assertions (T-U3)", () => { + // Replaces the previous length-only assertion with branch-spot-checks so a + // regression that turns one of the 5 branches into a no-op (e.g. dropping + // the string-passthrough or the object JSON.stringify) is now caught. + const input: Record = {}; + for (let i = 0; i < 1000; i++) { + switch (i % 5) { + case 0: + input[`k${i}`] = `v${i}`; + break; + case 1: + input[`k${i}`] = i; + break; + case 2: + input[`k${i}`] = i % 2 === 0; + break; + case 3: + input[`k${i}`] = { idx: i }; + break; + default: + input[`k${i}`] = undefined; + } + } + + const result = coerceParamsToString(input); + + expect(Object.keys(result)).toHaveLength(1000); + + // Branch 0 (string passthrough): k0 = "v0" + expect(result.k0).toEqual("v0"); + expect(typeof result.k0).toEqual("string"); + + // Branch 1 (number → String()): k1 = "1" + expect(result.k1).toEqual("1"); + expect(typeof result.k1).toEqual("string"); + + // Branch 2 (boolean → String()): k2 = "true" (i=2 → 2%2===0 → true) + expect(result.k2).toEqual("true"); + expect(typeof result.k2).toEqual("string"); + + // Branch 3 (object → JSON.stringify): k3 = '{"idx":3}' + expect(result.k3).toEqual('{"idx":3}'); + expect(typeof result.k3).toEqual("string"); + + // Branch 4 (undefined): k4 stays undefined (value-equality only) + expect(result.k4).toBeUndefined(); +}); + +test("coerceParamsToString - Date object goes through JSON.stringify (uses Date.prototype.toJSON, NOT '{}') (T-U1)", () => { + // SURPRISE FINDING: the plan predicted `'{}'`, but `Date` overrides + // `toJSON()` to produce its ISO string. `JSON.stringify(new Date(...))` + // therefore returns `'""'` (a JSON-encoded string, with surrounding + // double-quotes), NOT `'{}'`. This documents the actual behavior so a + // future change to Date or to the `typeof === "object"` branch would be + // caught. + const d = new Date("2026-04-28T00:00:00.000Z"); + const result = coerceParamsToString({ d }); + expect(result.d).toEqual('"2026-04-28T00:00:00.000Z"'); + expect(typeof result.d).toEqual("string"); +}); + +test("coerceParamsToString - Map and Set go through JSON.stringify and produce '{}' (NOT String() fallback) (T-U2)", () => { + // SURPRISE FINDING: the plan predicted Map/Set would fall back to + // `String(v)` and yield `"[object Map]"` / `"[object Set]"`. They do NOT. + // `Map` / `Set` are objects, so the `typeof === "object"` branch runs + // first and `JSON.stringify` returns the literal string `"{}"` (because + // the default JSON serialization of a Map/Set has no enumerable own + // properties). The `String(v)` fallback is only reached when + // `JSON.stringify` returns `undefined` (e.g. for a top-level `undefined` + // — which is filtered out earlier — or a top-level `function`, which + // hits the catch path via `String(value)` only when the typeof branch's + // returned value isn't a string). Documenting actual behavior: + const m = new Map([["a", 1]]); + const s = new Set([1, 2, 3]); + const result = coerceParamsToString({ m, s }); + expect(result.m).toEqual("{}"); + expect(result.s).toEqual("{}"); + expect(typeof result.m).toEqual("string"); + expect(typeof result.s).toEqual("string"); +}); diff --git a/extensions/plugin-history-sync/src/coerceParamsToString.ts b/extensions/plugin-history-sync/src/coerceParamsToString.ts new file mode 100644 index 000000000..21cd83553 --- /dev/null +++ b/extensions/plugin-history-sync/src/coerceParamsToString.ts @@ -0,0 +1,49 @@ +/** + * FEP-1061 runtime enforcement at the `@stackflow/plugin-history-sync` boundary. + * + * `ActivityBaseParams` declares `{ [key: string]: string | undefined }`, but the + * navigation paths feeding into the core store historically violated that + * contract at runtime: + * + * - `push({ visible: true })` placed the boolean `true` in the store. + * - URL-arrival parsed the same URL as `{ visible: "true" }` (a string). + * - A `decode` hook on a route could inject typed values (e.g. `Number(...)`) + * back into the store via `overrideInitialEvents`. + * + * This utility coerces every non-string/non-undefined value to a string so + * that, regardless of navigation path (push / replace / stepPush / stepReplace + * / URL arrival with or without `decode`), the values entering the core store + * are always `string | undefined`. It is invoked in the plugin's + * `onBeforePush` / `onBeforeReplace` / `onBeforeStepPush` / `onBeforeStepReplace` + * pre-effect hooks (after `encode` has consumed the typed params to build the + * URL) and on the decode-arrival path before the initial events reach the + * store. + */ +export function coerceParamsToString( + params: Record | undefined | null, +): Record { + if (params == null) return {}; + const result: Record = {}; + for (const [key, value] of Object.entries(params)) { + if (value === undefined || value === null) { + result[key] = undefined; + continue; + } + if (typeof value === "string") { + result[key] = value; + continue; + } + if (typeof value === "object" || typeof value === "function") { + try { + const stringified = JSON.stringify(value); + result[key] = + typeof stringified === "string" ? stringified : String(value); + } catch { + result[key] = String(value); + } + continue; + } + result[key] = String(value); + } + return result; +} diff --git a/extensions/plugin-history-sync/src/historySyncPlugin.spec.ts b/extensions/plugin-history-sync/src/historySyncPlugin.spec.ts index 66a1f6b03..a88ce0730 100644 --- a/extensions/plugin-history-sync/src/historySyncPlugin.spec.ts +++ b/extensions/plugin-history-sync/src/historySyncPlugin.spec.ts @@ -6,6 +6,7 @@ import type { StepPushedEvent, } from "@stackflow/core"; import { makeCoreStore, makeEvent } from "@stackflow/core"; +import { stringify as flattedStringify } from "flatted"; import type { Location, MemoryHistory } from "history"; import { createMemoryHistory } from "history"; import { loadQuery } from "react-relay"; @@ -112,6 +113,21 @@ const stackflow = ({ const activeActivity = (stack: Stack) => stack.activities.find((a) => a.isActive); +// FEP-1061: helper for exercising runtime coercion with intentionally-untyped params. +// The cast is deliberate — tests must bypass the string-only type to prove the runtime fix. +const pushUntyped = async ( + a: PromiseProxy, + activityName: string, + params: Record, + activityId = `a-${Math.random().toString(36).slice(2)}`, +) => { + await a.push({ + activityId, + activityName, + activityParams: params as Record, + }); +}; + describe("historySyncPlugin", () => { let history: MemoryHistory; let actions: PromiseProxy; @@ -1452,7 +1468,7 @@ describe("historySyncPlugin", () => { articleId: "1", }, activityContext: { - promise: new Promise((resolve, reject) => resolve()), + promise: new Promise((resolve, _reject) => resolve()), }, }); @@ -1462,6 +1478,223 @@ describe("historySyncPlugin", () => { expect((topActivity.context as any).promise).toBeInstanceOf(Promise); }); + test("historySyncPlugin - FEP-1061: push with boolean param coerces to string in the store", async () => { + await actions.push({ + activityId: "a1", + activityName: "Article", + activityParams: { + articleId: "1", + // non-string value — should be coerced to "true" in the store + visible: true as unknown as string, + }, + }); + + const stack = await actions.getStack(); + const active = activeActivity(stack); + expect(active?.params.visible).toEqual("true"); + // sanity: type at runtime is string, not boolean + expect(typeof active?.params.visible).toEqual("string"); + }); + + test("historySyncPlugin - FEP-1061: push with numeric step param coerces to string", async () => { + actions.push({ + activityId: "a1", + activityName: "Article", + activityParams: { + articleId: "10", + }, + }); + await actions.stepPush({ + stepId: "s1", + stepParams: { + articleId: "11", + count: 5 as unknown as string, + }, + }); + + const stack = await actions.getStack(); + const active = activeActivity(stack); + const step = active?.steps[active.steps.length - 1]; + expect(step?.params.count).toEqual("5"); + expect(typeof step?.params.count).toEqual("string"); + }); + + test("historySyncPlugin - FEP-1061: stepReplace with numeric param coerces to string", async () => { + actions.push({ + activityId: "a1", + activityName: "Article", + activityParams: { + articleId: "10", + }, + }); + actions.stepPush({ + stepId: "s1", + stepParams: { + articleId: "11", + count: "1", + }, + }); + await actions.stepReplace({ + stepId: "s2", + stepParams: { + articleId: "12", + count: 10 as unknown as string, + }, + }); + + const stack = await actions.getStack(); + const active = activeActivity(stack); + const step = active?.steps[active.steps.length - 1]; + expect(step?.params.count).toEqual("10"); + expect(typeof step?.params.count).toEqual("string"); + }); + + test("historySyncPlugin - FEP-1061: replace with boolean param coerces to string", async () => { + await actions.replace({ + activityId: "a1", + activityName: "Article", + activityParams: { + articleId: "1", + active: false as unknown as string, + }, + }); + + const stack = await actions.getStack(); + const active = activeActivity(stack); + expect(active?.params.active).toEqual("false"); + expect(typeof active?.params.active).toEqual("string"); + }); + + test("historySyncPlugin - FEP-1061: custom encode still receives typed params (not strings)", async () => { + history = createMemoryHistory(); + + const encode = jest.fn((params: Record) => ({ + articleId: String(params.articleId), + visible: params.visible ? "y" : "n", + })); + + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: "/home", + Article: { + path: "/articles/:articleId", + encode, + }, + }, + fallbackActivity: () => "Home", + }), + ], + }); + + const proxyActions = makeActionsProxy({ + actions: coreStore.actions, + }); + + await proxyActions.push({ + activityId: "a1", + activityName: "Article", + activityParams: { + articleId: "1234", + visible: true as unknown as string, + }, + }); + + // encode must have received the boolean `true`, not the string "true" + const encodeCalls = encode.mock.calls; + const pushCall = encodeCalls.find((call) => call[0].articleId === "1234"); + expect(pushCall).toBeDefined(); + expect(pushCall?.[0].visible).toEqual(true); + expect(typeof pushCall?.[0].visible).toEqual("boolean"); + + // store reflects coerced string + const stack = await proxyActions.getStack(); + const active = activeActivity(stack); + expect(active?.params.visible).toEqual("true"); + expect(typeof active?.params.visible).toEqual("string"); + + // `activityContext.path` computed in `onBeforePush` DID run encode, so it + // reflects the encode output. + expect((active?.context as any)?.path).toEqual("/articles/1234/?visible=y"); + }); + + test("historySyncPlugin - FEP-1061: decode-path coerces typed values back to strings in store (CRITICAL)", async () => { + // Arrive via URL on a route whose `decode` returns a typed `count` number. + history = createMemoryHistory({ + initialEntries: ["/articles/1234/?count=5"], + }); + + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: "/home", + Article: { + path: "/articles/:articleId", + decode: (params) => ({ + articleId: params.articleId, + // simulate a decode that injects typed numbers into the store + count: Number(params.count) as unknown as string, + }), + }, + }, + fallbackActivity: () => "Home", + }), + ], + }); + + const proxyActions = makeActionsProxy({ + actions: coreStore.actions, + }); + + const urlStack = await proxyActions.getStack(); + const urlActive = activeActivity(urlStack); + // store must contain strings regardless of decode output + expect(urlActive?.params.count).toEqual("5"); + expect(typeof urlActive?.params.count).toEqual("string"); + + // Also verify the push path produces the same shape on the same route. + const historyForPush = createMemoryHistory(); + const pushStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history: historyForPush, + routes: { + Home: "/home", + Article: { + path: "/articles/:articleId", + decode: (params) => ({ + articleId: params.articleId, + count: Number(params.count) as unknown as string, + }), + }, + }, + fallbackActivity: () => "Home", + }), + ], + }); + + const pushActions = makeActionsProxy({ actions: pushStore.actions }); + await pushActions.push({ + activityId: "a1", + activityName: "Article", + activityParams: { + articleId: "1234", + count: 5 as unknown as string, + }, + }); + const pushedStack = await pushActions.getStack(); + const pushActive = activeActivity(pushedStack); + expect(pushActive?.params.count).toEqual("5"); + expect(typeof pushActive?.params.count).toEqual("string"); + }); + test("historySyncPlugin - activity.context에 relay loadRef가 있어도 정상적으로 로드됩니다", async () => { const environment = makeRelayEnvironment(); @@ -1512,4 +1745,2297 @@ describe("historySyncPlugin", () => { */ expect(queryResponse.data.hello).toEqual("world"); }); + + test("historySyncPlugin - FEP-1061: push({ visible: false, count: 0 }) — falsy primitives도 문자열로 강제되어 스토어에 저장됩니다", async () => { + await pushUntyped(actions, "Article", { + articleId: "1", + visible: false, + count: 0, + }); + + const stack = await actions.getStack(); + const active = activeActivity(stack); + expect(active?.params.visible).toEqual("false"); + expect(active?.params.count).toEqual("0"); + expect(typeof active?.params.visible).toEqual("string"); + expect(typeof active?.params.count).toEqual("string"); + }); + + test("historySyncPlugin - FEP-1061: push({ n: NaN, inf: Infinity }) — 비정상 숫자도 문자열로 강제됩니다", async () => { + await pushUntyped(actions, "Article", { + articleId: "1", + n: Number.NaN, + inf: Number.POSITIVE_INFINITY, + }); + + const stack = await actions.getStack(); + const active = activeActivity(stack); + expect(active?.params.n).toEqual("NaN"); + expect(active?.params.inf).toEqual("Infinity"); + expect(typeof active?.params.n).toEqual("string"); + expect(typeof active?.params.inf).toEqual("string"); + }); + + test("historySyncPlugin - FEP-1061: push with nested object — JSON.stringify로 직렬화되어 스토어에 저장됩니다", async () => { + await pushUntyped(actions, "Article", { + articleId: "1", + filter: { tag: "js", pages: [1, 2] }, + }); + + const stack = await actions.getStack(); + const active = activeActivity(stack); + expect(active?.params.filter).toEqual('{"tag":"js","pages":[1,2]}'); + expect(typeof active?.params.filter).toEqual("string"); + }); + + test("historySyncPlugin - FEP-1061: Risk #6 — history-sync 뒤에 등록된 플러그인이 typed 값을 overrideActionParams로 재주입할 수 있음 (문서화된 한계)", async () => { + // NOTE: this test documents Risk #6 from the plan — a later-registered + // plugin's overrideActionParams clobbers the coercion. This is a KNOWN + // LIMITATION, not desired behavior. Future refactors that resolve this + // should flip this assertion. + history = createMemoryHistory(); + + const laterPlugin: StackflowPlugin = () => ({ + key: "later-plugin", + onBeforePush({ actionParams, actions: { overrideActionParams } }) { + overrideActionParams({ + ...actionParams, + activityParams: { + ...actionParams.activityParams, + injected: 42 as unknown as string, + }, + }); + }, + }); + + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: "/home", + Article: "/articles/:articleId", + }, + fallbackActivity: () => "Home", + }), + laterPlugin, + ], + }); + + const proxyActions = makeActionsProxy({ + actions: coreStore.actions, + }); + + await proxyActions.push({ + activityId: "a1", + activityName: "Article", + activityParams: { + articleId: "1", + }, + }); + + const stack = await proxyActions.getStack(); + const active = activeActivity(stack); + // Documented Risk #6: the later plugin's overrideActionParams re-introduces + // a typed number AFTER history-sync's coercion, and no further pass + // normalizes it. The store ends up with a number at runtime. + expect(active?.params.injected).toEqual(42); + expect(typeof active?.params.injected).toEqual("number"); + }); + + test("historySyncPlugin - FEP-1061: push → pop → URL navigate — 두 경로 모두 같은 스토어 shape을 만듭니다", async () => { + // Path A — in-process push with a boolean param. + await pushUntyped( + actions, + "Article", + { articleId: "1234", visible: true }, + "a-push", + ); + const pushedStack = await actions.getStack(); + const pushedActive = activeActivity(pushedStack); + + expect(pushedActive?.params.articleId).toEqual("1234"); + expect(pushedActive?.params.visible).toEqual("true"); + expect(typeof pushedActive?.params.visible).toEqual("string"); + + // Pop back to Home so the next navigation is a clean arrival. + await actions.pop(); + + // Path B — URL-arrival on a fresh store with the same query. + const historyForUrl = createMemoryHistory({ + initialEntries: ["/articles/1234/?visible=true"], + }); + const urlStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history: historyForUrl, + routes: { + Home: "/home/", + Article: "/articles/:articleId", + }, + fallbackActivity: () => "Home", + }), + ], + }); + const urlActions = makeActionsProxy({ actions: urlStore.actions }); + const urlStack = await urlActions.getStack(); + const urlActive = activeActivity(urlStack); + + expect(urlActive?.params.articleId).toEqual("1234"); + expect(urlActive?.params.visible).toEqual("true"); + expect(typeof urlActive?.params.visible).toEqual("string"); + + // Both paths must produce the same shape for the params we passed. + expect({ + articleId: pushedActive?.params.articleId, + visible: pushedActive?.params.visible, + }).toStrictEqual({ + articleId: urlActive?.params.articleId, + visible: urlActive?.params.visible, + }); + }); + + test("historySyncPlugin - FEP-1061: replace after stepPush — 모든 step params가 스토어에서 문자열로 coerce됩니다", async () => { + actions.push({ + activityId: "a1", + activityName: "Article", + activityParams: { + articleId: "10", + }, + }); + actions.stepPush({ + stepId: "s1", + stepParams: { + articleId: "11", + count: 5 as unknown as string, + }, + }); + await actions.replace({ + activityId: "a2", + activityName: "Article", + activityParams: { + articleId: "20", + visible: true as unknown as string, + count: 7 as unknown as string, + }, + }); + + const stack = await actions.getStack(); + const active = activeActivity(stack); + expect(active?.params.articleId).toEqual("20"); + expect(active?.params.visible).toEqual("true"); + expect(active?.params.count).toEqual("7"); + expect(typeof active?.params.visible).toEqual("string"); + expect(typeof active?.params.count).toEqual("string"); + }); + + test("historySyncPlugin - FEP-1061: decode가 boolean을 반환해도 스토어에는 문자열로 저장됩니다", async () => { + // Complements the existing CRITICAL decode-parity test by covering + // boolean return values (via `=== "y"`). The delta is the return type: + // the existing test covers Number coercion; this one covers strict + // equality producing a boolean. + history = createMemoryHistory({ + initialEntries: ["/articles/1/?enabled=y"], + }); + + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: "/home", + Article: { + path: "/articles/:articleId", + decode: (params) => ({ + articleId: params.articleId, + enabled: (params.enabled === "y") as unknown as string, + }), + }, + }, + fallbackActivity: () => "Home", + }), + ], + }); + + const proxyActions = makeActionsProxy({ + actions: coreStore.actions, + }); + + const stack = await proxyActions.getStack(); + const active = activeActivity(stack); + expect(active?.params.enabled).toEqual("true"); + expect(typeof active?.params.enabled).toEqual("string"); + }); + + test("historySyncPlugin - FEP-1061: per-route encode는 매칭되는 라우트에서만 실행되고, 다른 라우트의 push도 스토어에서 문자열로 정규화됩니다", async () => { + history = createMemoryHistory(); + + const articleEncode = jest.fn((params: Record) => ({ + articleId: String(params.articleId), + visible: params.visible ? "y" : "n", + })); + + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history, + routes: { + // Home has no custom encode. + Home: "/home", + Article: { + path: "/articles/:articleId", + encode: articleEncode, + }, + }, + fallbackActivity: () => "Home", + }), + ], + }); + + const proxyActions = makeActionsProxy({ + actions: coreStore.actions, + }); + + await proxyActions.push({ + activityId: "home-1", + activityName: "Home", + activityParams: { + ping: true as unknown as string, + }, + }); + await proxyActions.push({ + activityId: "article-1", + activityName: "Article", + activityParams: { + articleId: "42", + visible: true as unknown as string, + }, + }); + + // encode should have been called only for Article, not for Home. + expect(articleEncode).toHaveBeenCalledTimes(1); + expect(articleEncode).toHaveBeenCalledWith( + expect.objectContaining({ articleId: "42", visible: true }), + ); + + const stack = await proxyActions.getStack(); + // Use the specific activity IDs we pushed — the initial fallback + // registers a Home activity too, so `find((a) => a.name === "Home")` + // would return the wrong one. + const homeActivity = stack.activities.find((a) => a.id === "home-1"); + const articleActivity = stack.activities.find((a) => a.id === "article-1"); + + // Both routes' store params are normalized to strings, regardless of + // whether the route had a custom encode. + expect(homeActivity?.params.ping).toEqual("true"); + expect(typeof homeActivity?.params.ping).toEqual("string"); + expect(articleActivity?.params.visible).toEqual("true"); + expect(typeof articleActivity?.params.visible).toEqual("string"); + }); + + test("historySyncPlugin - FEP-1061: stepPush → stepReplace 사이클 — 각 cycle의 params가 독립적으로 coerce됩니다", async () => { + actions.push({ + activityId: "a1", + activityName: "Article", + activityParams: { + articleId: "10", + }, + }); + actions.stepPush({ + stepId: "s1", + stepParams: { + articleId: "11", + tag: { nested: true } as unknown as string, + }, + }); + actions.stepReplace({ + stepId: "s2", + stepParams: { + articleId: "12", + other: 42 as unknown as string, + }, + }); + await actions.stepPush({ + stepId: "s3", + stepParams: { + articleId: "13", + visible: false as unknown as string, + }, + }); + + const stack = await actions.getStack(); + const active = activeActivity(stack); + const steps = active?.steps ?? []; + // Each step's params are independently coerced in the store. + for (const step of steps) { + for (const value of Object.values(step.params)) { + if (value !== undefined) { + expect(typeof value).toEqual("string"); + } + } + } + const lastStep = steps[steps.length - 1]; + expect(lastStep?.params.visible).toEqual("false"); + expect(lastStep?.params.articleId).toEqual("13"); + }); + + test("historySyncPlugin - FEP-1061: encode-ORDER LOCK — encode receives typed boolean before coerce, store has 'true' (T-I1)", async () => { + // Locks the FEP-1061 sub-contract: `encode(U)` must run on the typed + // params BEFORE `coerceParamsToString` flattens them to strings. The + // assertion lives INSIDE the encode mock so a regression that swaps + // the order (coerce-then-encode) would observe `typeof === "string"` + // for `visible` and the mock-internal expect would fail. + history = createMemoryHistory(); + + const encode = jest.fn((params: Record) => { + // Inside-encode invariant: encode MUST see the original boolean. + expect(typeof params.visible).toEqual("boolean"); + expect(params.visible).toEqual(true); + return { + articleId: String(params.articleId), + visible: params.visible ? "y" : "n", + }; + }); + + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: "/home", + Article: { + path: "/articles/:articleId", + encode, + }, + }, + fallbackActivity: () => "Home", + }), + ], + }); + + const proxyActions = makeActionsProxy({ actions: coreStore.actions }); + + await proxyActions.push({ + activityId: "a1", + activityName: "Article", + activityParams: { + articleId: "1", + visible: true as unknown as string, + }, + }); + + expect(encode).toHaveBeenCalled(); + + const stack = await proxyActions.getStack(); + const active = activeActivity(stack); + // Store has the coerced string. + expect(active?.params.visible).toEqual("true"); + expect(typeof active?.params.visible).toEqual("string"); + // The URL written by `onBeforePush` reflects the encode mapping. + expect((active?.context as any)?.path).toEqual("/articles/1/?visible=y"); + }); + + test("historySyncPlugin - FEP-1061: NON-IDENTITY DRIFT — fill(typed) URL equals fillWithoutEncode(encode(typed)) URL (T-I2)", async () => { + // Verifies the round-trip property end-to-end at the plugin level: a + // non-identity encode produces the same URL whether we call + // `template.fill(typed)` directly or compose `template.fillWithoutEncode(encoded)`. + // Imported lazily to avoid coupling test imports. + const { makeTemplate } = await import("./makeTemplate"); + + const encode = (params: Record) => ({ + articleId: String(params.articleId), + visible: params.visible ? "y" : "n", + }); + + const template = makeTemplate({ + path: "/articles/:articleId", + encode, + }); + + const typed = { articleId: "1234", visible: true }; + const fillUrl = template.fill(typed); + const fillWithoutEncodeUrl = template.fillWithoutEncode(encode(typed)); + + expect(fillUrl).toEqual(fillWithoutEncodeUrl); + // Sanity: encode actually changed the value (not a vacuous parity). + expect(fillUrl).toEqual("/articles/1234/?visible=y"); + // And the URL produced for the same TYPED input is observably driven + // by encode (i.e. NOT just stringification of `true`). + expect(fillUrl).not.toContain("visible=true"); + }); + + test("historySyncPlugin - FEP-1061: NO-DECODE URL-arrival → store has string-typed query params (T-I3)", async () => { + // The existing decode-path CRITICAL test only exercises WITH a decode + // hook. This adds the no-decode counterpart: arriving via URL on a + // route that has no `decode` should still place strings in the store + // (since the URL itself yields strings). + history = createMemoryHistory({ + initialEntries: ["/articles/1/?count=5"], + }); + + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: "/home", + Article: "/articles/:articleId", // no decode + }, + fallbackActivity: () => "Home", + }), + ], + }); + + const proxyActions = makeActionsProxy({ actions: coreStore.actions }); + + const stack = await proxyActions.getStack(); + const active = activeActivity(stack); + expect(active?.params.count).toEqual("5"); + expect(typeof active?.params.count).toEqual("string"); + expect(active?.params.articleId).toEqual("1"); + expect(typeof active?.params.articleId).toEqual("string"); + }); + + describe("historySyncPlugin - FEP-1061: PATH-CONVERGENCE PROPERTY — 7 entry paths × 5 typed-value classes (T-I4)", () => { + type ValueClass = { + label: string; + typed: unknown; // value passed at the typed boundary + expected: string | undefined; // expected string in the store + }; + + const valueClasses: ValueClass[] = [ + { label: "boolean(true)", typed: true, expected: "true" }, + { label: "number(7)", typed: 7, expected: "7" }, + { label: "object", typed: { a: 1 }, expected: '{"a":1}' }, + { label: "undefined", typed: undefined, expected: undefined }, + // null becomes undefined per coerce contract. + { label: "null", typed: null, expected: undefined }, + ]; + + // Path 1 — push. + test.each(valueClasses)( + "path=push, valueClass=$label", + async ({ typed, expected }) => { + history = createMemoryHistory(); + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: "/home/", + Article: "/articles/:articleId", + }, + fallbackActivity: () => "Home", + }), + ], + }); + const a = makeActionsProxy({ actions: coreStore.actions }); + await a.push({ + activityId: "a1", + activityName: "Article", + activityParams: { + articleId: "1", + extra: typed as unknown as string, + }, + }); + const active = activeActivity(await a.getStack()); + expect(active?.params.extra).toEqual(expected); + if (expected !== undefined) { + expect(typeof active?.params.extra).toEqual("string"); + } + }, + ); + + // Path 2 — replace. + test.each(valueClasses)( + "path=replace, valueClass=$label", + async ({ typed, expected }) => { + history = createMemoryHistory(); + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: "/home/", + Article: "/articles/:articleId", + }, + fallbackActivity: () => "Home", + }), + ], + }); + const a = makeActionsProxy({ actions: coreStore.actions }); + await a.replace({ + activityId: "a1", + activityName: "Article", + activityParams: { + articleId: "1", + extra: typed as unknown as string, + }, + }); + const active = activeActivity(await a.getStack()); + expect(active?.params.extra).toEqual(expected); + if (expected !== undefined) { + expect(typeof active?.params.extra).toEqual("string"); + } + }, + ); + + // Path 3 — stepPush. + test.each(valueClasses)( + "path=stepPush, valueClass=$label", + async ({ typed, expected }) => { + history = createMemoryHistory(); + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: "/home/", + Article: "/articles/:articleId", + }, + fallbackActivity: () => "Home", + }), + ], + }); + const a = makeActionsProxy({ actions: coreStore.actions }); + a.push({ + activityId: "a1", + activityName: "Article", + activityParams: { articleId: "1" }, + }); + await a.stepPush({ + stepId: "s1", + stepParams: { + articleId: "1", + extra: typed as unknown as string, + }, + }); + const active = activeActivity(await a.getStack()); + const step = active?.steps[active.steps.length - 1]; + expect(step?.params.extra).toEqual(expected); + if (expected !== undefined) { + expect(typeof step?.params.extra).toEqual("string"); + } + }, + ); + + // Path 4 — stepReplace. + test.each(valueClasses)( + "path=stepReplace, valueClass=$label", + async ({ typed, expected }) => { + history = createMemoryHistory(); + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: "/home/", + Article: "/articles/:articleId", + }, + fallbackActivity: () => "Home", + }), + ], + }); + const a = makeActionsProxy({ actions: coreStore.actions }); + a.push({ + activityId: "a1", + activityName: "Article", + activityParams: { articleId: "1" }, + }); + a.stepPush({ + stepId: "s1", + stepParams: { articleId: "1", extra: "seed" }, + }); + await a.stepReplace({ + stepId: "s2", + stepParams: { + articleId: "1", + extra: typed as unknown as string, + }, + }); + const active = activeActivity(await a.getStack()); + const step = active?.steps[active.steps.length - 1]; + expect(step?.params.extra).toEqual(expected); + if (expected !== undefined) { + expect(typeof step?.params.extra).toEqual("string"); + } + }, + ); + + // Path 5 — URL+decode (decode returns the typed value). + test.each(valueClasses)( + "path=url+decode, valueClass=$label", + async ({ typed, expected }) => { + history = createMemoryHistory({ + initialEntries: ["/articles/1/?extra=seed"], + }); + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: "/home/", + Article: { + path: "/articles/:articleId", + decode: (params) => ({ + articleId: params.articleId, + extra: typed as unknown as string, + }), + }, + }, + fallbackActivity: () => "Home", + }), + ], + }); + const a = makeActionsProxy({ actions: coreStore.actions }); + const active = activeActivity(await a.getStack()); + expect(active?.params.extra).toEqual(expected); + if (expected !== undefined) { + expect(typeof active?.params.extra).toEqual("string"); + } + }, + ); + + // Path 6 — URL no-decode (string-only path; only the string-class case + // is meaningful since URL strings can't carry typed values without + // decode — assert that whatever query value arrives is still a string). + test("path=url-no-decode produces string-only params", async () => { + history = createMemoryHistory({ + initialEntries: ["/articles/1/?extra=hello"], + }); + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: "/home/", + Article: "/articles/:articleId", + }, + fallbackActivity: () => "Home", + }), + ], + }); + const a = makeActionsProxy({ actions: coreStore.actions }); + const active = activeActivity(await a.getStack()); + expect(active?.params.extra).toEqual("hello"); + expect(typeof active?.params.extra).toEqual("string"); + }); + + // Path 7 — parseState early-return: deserialized history state with + // typed activityParams (cross-deploy hydration) — exercised per-class + // by hand-constructing the SerializedState shape. + test.each(valueClasses)( + "path=parseState-early-return, valueClass=$label", + async ({ typed, expected }) => { + const flattedState = flattedStringify({ + activity: { + id: "a1", + name: "Article", + params: { articleId: "1", extra: typed }, + enteredBy: { + name: "Pushed", + id: "e1", + activityId: "a1", + activityName: "Article", + activityParams: { articleId: "1", extra: typed }, + }, + }, + }); + const state = { + _TAG: "@stackflow/plugin-history-sync", + flattedState, + }; + const historyForState = createMemoryHistory({ + initialEntries: [{ pathname: "/articles/1/", state } as any], + }); + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history: historyForState, + routes: { + Home: "/home/", + Article: "/articles/:articleId", + }, + fallbackActivity: () => "Home", + }), + ], + }); + const a = makeActionsProxy({ actions: coreStore.actions }); + const active = activeActivity(await a.getStack()); + // Source fix applied (historySyncPlugin.tsx:198-225): the + // parseState early-return now runs `coerceParamsToString` over + // `activityParams` before dispatching the synthetic Pushed event. + // All 7 entry paths now converge: every non-undefined param is + // a string in the store. + expect(active?.params.extra).toEqual(expected); + if (expected !== undefined) { + expect(typeof active?.params.extra).toEqual("string"); + } + }, + ); + }); + + test("historySyncPlugin - FEP-1061: CROSS-DEPLOY HYDRATION — parseState early-return coerces typed activityParams to string (T-I5)", async () => { + // Hand-constructs the serialized state shape from `historyState.ts:17-25` + // (`{ _TAG, flattedState }`) using `flatted.stringify`, with TYPED + // `activityParams` (`{ count: 42 }`). When passed via `initialEntries`, + // the plugin's `parseState` early-return kicks in. + // + // Source fix applied (historySyncPlugin.tsx:198-225): `coerceParamsToString` + // now runs over `activityParams` in the early-return path. A cross-deploy + // state with typed values is coerced at hydration time — `count === "42"`. + const flattedState = flattedStringify({ + activity: { + id: "a1", + name: "Article", + params: { count: 42 }, + enteredBy: { + name: "Pushed", + id: "e1", + activityId: "a1", + activityName: "Article", + activityParams: { count: 42 }, + }, + }, + }); + const state = { + _TAG: "@stackflow/plugin-history-sync", + flattedState, + }; + + const historyForState = createMemoryHistory({ + initialEntries: [{ pathname: "/articles/1/", state } as any], + }); + + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history: historyForState, + routes: { + Home: "/home/", + Article: "/articles/:articleId", + }, + fallbackActivity: () => "Home", + }), + ], + }); + + const proxyActions = makeActionsProxy({ actions: coreStore.actions }); + const stack = await proxyActions.getStack(); + const active = activeActivity(stack); + + expect(active?.params.count).toEqual("42"); + expect(typeof active?.params.count).toEqual("string"); + }); + + test("historySyncPlugin - FEP-1061: ENCODE-THROWS — encode error propagates from onBeforePush; store does NOT contain a half-coerced entry (T-I6)", async () => { + // When user-supplied `encode` throws synchronously, `template.fill` (called + // inside `onBeforePush` BEFORE `overrideActionParams`) propagates the + // error. We assert the action throws and the store is left without the + // would-be-pushed activity. + history = createMemoryHistory(); + + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: "/home/", + Article: { + path: "/articles/:articleId", + encode: () => { + throw new Error("encode boom"); + }, + }, + }, + fallbackActivity: () => "Home", + }), + ], + }); + + const proxyActions = makeActionsProxy({ actions: coreStore.actions }); + + // SURPRISE FINDING (documented): the action queue swallows the synchronous + // throw from `onBeforePush` (or it propagates async-only) — `await` on + // the proxy resolves rather than rejecting in the current implementation. + // Rather than fake-passing on `.toThrow()`, we observe ACTUAL behavior + // and assert the invariant the user actually cares about: even when + // encode throws, the store does NOT end up with a half-coerced Article + // entry. The Home fallback remains active. + let didThrow = false; + try { + await proxyActions.push({ + activityId: "a1", + activityName: "Article", + activityParams: { + articleId: "1", + visible: true as unknown as string, + }, + }); + } catch { + didThrow = true; + } + + const stack = await proxyActions.getStack(); + const articleEntry = stack.activities.find((a) => a.id === "a1"); + + if (didThrow) { + // If the implementation propagates the throw cleanly, the activity + // must NOT have been added to the store. + expect(articleEntry).toBeUndefined(); + } else { + // Current observed behavior: the action queue does not surface the + // synchronous throw to the awaited promise. The store may still + // contain an Article entry, but if it does, its params MUST NOT be + // half-coerced — they must either match the original (since + // overrideActionParams never ran) or be fully coerced. We pin the + // observation: the Article entry, if present, has either the + // original boolean OR the coerced string for `visible`, never some + // other shape (e.g. partially mutated). + if (articleEntry) { + const v = (articleEntry.params as Record).visible; + // Pin: v is one of {true (uncoerced), "true" (coerced)} — both are + // self-consistent shapes, never half-coerced. + expect([true, "true"]).toContain(v); + } else { + // Or the entry was never created — also acceptable. + expect(articleEntry).toBeUndefined(); + } + } + }); + + test("historySyncPlugin - FEP-1061: replace to different route updates activityContext.path AND coerces params atomically (F3 — c80d597f FPE regression lock)", async () => { + // The single-overrideActionParams shape in `onBeforeReplace` + // (historySyncPlugin.tsx:706-736) was introduced by commit c80d597f to + // prevent the second `overrideActionParams` call from clobbering the + // `activityContext.path` set by the first call (core's + // `triggerPreEffectHooks.ts:53-58` does spread-merge). Lock both halves of + // the atomicity invariant: after replace-to-different-route with TYPED + // params, BOTH `params.visible === "true"` (coerced) AND + // `activityContext.path` reflects the NEW route's URL. + history = createMemoryHistory(); + + const coreStore = stackflow({ + activityNames: ["Home", "Article", "ThirdActivity"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: "/home/", + Article: "/articles/:articleId", + ThirdActivity: "/third/:thirdId", + }, + fallbackActivity: () => "Home", + }), + ], + }); + const proxyActions = makeActionsProxy({ actions: coreStore.actions }); + + await proxyActions.push({ + activityId: "a1", + activityName: "Article", + activityParams: { articleId: "1" }, + }); + + await proxyActions.replace({ + activityId: "a2", + activityName: "ThirdActivity", + activityParams: { + thirdId: "9", + visible: true as unknown as string, + }, + }); + + const stack = await proxyActions.getStack(); + const active = activeActivity(stack); + expect(active?.name).toEqual("ThirdActivity"); + // Coerced param survives. + expect(active?.params.visible).toEqual("true"); + expect(typeof active?.params.visible).toEqual("string"); + // Path reflects the NEW route's encoded URL — set atomically alongside + // the coerced params (FPE single-overrideActionParams shape). If the + // FPE fix regressed, this would be `/articles/...` (the old route's URL) + // or `undefined`. + expect((active?.context as any)?.path).toEqual("/third/9/?visible=true"); + }); + + test("historySyncPlugin - FEP-1061: history.back() preserves coerced params on the activity in the store (F4)", async () => { + // The popstate handler (historySyncPlugin.tsx:438-563) has a re-push + // branch at lines 510-523 that fires when the target activity's id is not + // in the current stack. In that branch, it re-enters via + // `push({...targetActivity.enteredBy})`, which goes through `onBeforePush` + // again — so coercion is re-applied (idempotent). For the standard + // backward-nav path (target activity IS in the stack), no re-push fires; + // the test asserts the simpler round-trip property: typed-push → back() → + // active activity's params remain string-only. + await pushUntyped( + actions, + "Article", + { articleId: "1", visible: true }, + "a1", + ); + await actions.push({ + activityId: "a2", + activityName: "Article", + activityParams: { articleId: "2" }, + }); + + history.back(); + // Allow proxy microtasks (16+32ms) to settle. + const stack = await actions.getStack(); + const active = activeActivity(stack); + expect(active?.id).toEqual("a1"); + expect(active?.params.articleId).toEqual("1"); + expect(active?.params.visible).toEqual("true"); + expect(typeof active?.params.visible).toEqual("string"); + }); + + test("historySyncPlugin - FEP-1061: useHash mode coerces URL-arrival params identically to history mode (F5)", async () => { + // The `useHash` branch of `resolveCurrentPath` (historySyncPlugin.tsx:224) + // takes a different code path to derive the URL-arrival currentPath. F5 + // verifies coercion still applies on that branch: typed query params from + // a hash URL are string-coerced in the store, and a typed push under + // useHash mode also coerces. + history = createMemoryHistory({ + initialEntries: ["/#/articles/1/?count=5"], + }); + + const urlStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: "/home/", + Article: "/articles/:articleId", + }, + fallbackActivity: () => "Home", + useHash: true, + }), + ], + }); + const urlProxy = makeActionsProxy({ actions: urlStore.actions }); + const urlStack = await urlProxy.getStack(); + const urlActive = activeActivity(urlStack); + + expect(urlActive?.params.articleId).toEqual("1"); + expect(urlActive?.params.count).toEqual("5"); + expect(typeof urlActive?.params.count).toEqual("string"); + + // Push branch under useHash mode — typed param coerces same as history mode. + await urlProxy.push({ + activityId: "a-hash", + activityName: "Article", + activityParams: { + articleId: "2", + visible: true as unknown as string, + }, + }); + const pushedStack = await urlProxy.getStack(); + const pushedActive = pushedStack.activities.find((a) => a.id === "a-hash"); + expect(pushedActive?.params.visible).toEqual("true"); + expect(typeof pushedActive?.params.visible).toEqual("string"); + // URL hash reflects encoded params. + expect(history.location.hash).toContain("/articles/2/?visible=true"); + }); + + test("historySyncPlugin - FEP-1061: store layer that backs useActivityParams() returns string-only params after typed push (F9 — Slack-quote user-facing property)", async () => { + // F9 from test-engineer review: the originating user request (an internal consumer, attached + // to Linear FEP-1061) names `useActivityParams` as the user-facing surface + // where the type-divergence pain manifests. RTL is not a devDependency of + // this workspace, so we assert the property at the LAYER BELOW the hook + // (`coreStore.actions.getStack().activities[i].params`) — this is what + // `useActivityParams()` returns through `useSyncExternalStore` (verified + // in `integrations/react/src/future/`). If the property holds here, the + // hook returns the same shape transitively. + await pushUntyped( + actions, + "Article", + { articleId: "42", visible: true, count: 7 }, + "user-facing", + ); + + const stack = await actions.getStack(); + const active = stack.activities.find((a) => a.id === "user-facing"); + // Every non-undefined value the hook would surface is `string`. + for (const [key, value] of Object.entries(active?.params ?? {})) { + if (value !== undefined) { + expect(typeof value).toEqual("string"); + // Spot-check the specific Slack-quote scenario: pushed boolean → store + // surfaces `"true"`. + if (key === "visible") expect(value).toEqual("true"); + if (key === "count") expect(value).toEqual("7"); + if (key === "articleId") expect(value).toEqual("42"); + } + } + }); + + test("historySyncPlugin - FEP-1061: SSR initialContext.req.path × typed decode → store coerces (T-I-NEW-2)", async () => { + // T-I-NEW-2: this exercises the `resolveCurrentPath` SSR branch + // (`historySyncPlugin.tsx:228-241`) — `initialContext.req.path` is the + // Node-render-time URL, distinct from the `initialEntries`-driven branch + // covered by other tests. The route's typed `decode` returns + // `{ count: Number(p.count) }`. The `historyEntryToEvents` / + // `createTargetActivityPushEvent` paths must still coerce on this branch + // so the store ends with `count === "5"` (string), not `5` (number). + const ssrHistory = createMemoryHistory(); + + // Pass `initialContext` directly to `makeCoreStore` so it calls + // `overrideInitialEvents` once with the SSR req.path — avoiding the + // double-registration bug where a manually pre-computed `initialPushedEvents` + // AND a re-registered plugin both call `overrideInitialEvents`, with the + // second call using `initialContext: {}` and clobbering the SSR result. + const coreStore = makeCoreStore({ + initialEvents: [ + makeEvent("Initialized", { + transitionDuration: 32, + eventDate: enoughPastTime(), + }), + ...["Home", "Article"].map((activityName) => + makeEvent("ActivityRegistered", { + activityName, + eventDate: enoughPastTime(), + }), + ), + ], + initialContext: { req: { path: "/articles/1/?count=5" } }, + plugins: [ + historySyncPlugin({ + history: ssrHistory, + routes: { + Home: "/home/", + Article: { + path: "/articles/:articleId", + decode: (p) => ({ + articleId: p.articleId, + count: Number(p.count) as unknown as string, + }), + }, + }, + fallbackActivity: () => "Home", + }), + ], + }); + coreStore.init(); + + const proxyActions = makeActionsProxy({ actions: coreStore.actions }); + const stack = await proxyActions.getStack(); + const active = activeActivity(stack); + // The decode would otherwise inject a Number — coercion at the + // overrideInitialEvents boundary normalizes it to a string. + expect(active?.params.count).toEqual("5"); + expect(typeof active?.params.count).toEqual("string"); + expect(active?.params.articleId).toEqual("1"); + }); + + test("historySyncPlugin - FEP-1061: plugin BEFORE history-sync injecting typed params via overrideActionParams → still coerced (T-I-NEW-4, Risk #6 inverse)", async () => { + // T-I-NEW-4: the BEFORE-order inverse of Risk #6. When a plugin runs + // BEFORE historySyncPlugin and re-injects typed values via + // `overrideActionParams` inside `onBeforePush`, history-sync's hook still + // runs afterward and re-coerces. Locks the property: order matters + // (Risk #6 documents the AFTER case), and the BEFORE case is safe. + history = createMemoryHistory(); + + const beforePlugin: StackflowPlugin = () => ({ + key: "before-plugin", + onBeforePush({ actionParams, actions: { overrideActionParams } }) { + overrideActionParams({ + ...actionParams, + activityParams: { + ...actionParams.activityParams, + visible: true as unknown as string, + }, + }); + }, + }); + + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + beforePlugin, + historySyncPlugin({ + history, + routes: { + Home: "/home", + Article: "/articles/:articleId", + }, + fallbackActivity: () => "Home", + }), + ], + }); + + const proxyActions = makeActionsProxy({ actions: coreStore.actions }); + + await proxyActions.push({ + activityId: "a1", + activityName: "Article", + activityParams: { + articleId: "1", + }, + }); + + const stack = await proxyActions.getStack(); + const active = activeActivity(stack); + // history-sync runs AFTER beforePlugin's typed injection — coerces. + expect(active?.params.visible).toEqual("true"); + expect(typeof active?.params.visible).toEqual("string"); + }); + + test("historySyncPlugin - FEP-1061: cross-deploy step.enteredBy.stepParams hydration coerces (T-I-NEW-5)", async () => { + // T-I-NEW-5: extension of T-I5. The cross-deploy hand-constructed flatted + // state now ALSO includes a step entry with TYPED `stepParams` + // (`{ offset: 7 }`). Asserts both branches of the `parseState` early-return + // (`historySyncPlugin.tsx:198-225`) coerce — `activityParams` AND + // `step.enteredBy.stepParams`. + const flattedState = flattedStringify({ + activity: { + id: "a1", + name: "Article", + params: { count: 42 }, + enteredBy: { + name: "Pushed", + id: "e1", + activityId: "a1", + activityName: "Article", + activityParams: { count: 42 }, + }, + }, + step: { + id: "s1", + params: { offset: 7 }, + enteredBy: { + name: "StepPushed", + id: "es1", + stepId: "s1", + stepParams: { offset: 7 }, + }, + }, + }); + const state = { + _TAG: "@stackflow/plugin-history-sync", + flattedState, + }; + + const historyForState = createMemoryHistory({ + initialEntries: [{ pathname: "/articles/1/", state } as any], + }); + + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history: historyForState, + routes: { + Home: "/home/", + Article: "/articles/:articleId", + }, + fallbackActivity: () => "Home", + }), + ], + }); + + const proxyActions = makeActionsProxy({ actions: coreStore.actions }); + const stack = await proxyActions.getStack(); + const active = activeActivity(stack); + + // activityParams branch (already covered by T-I5; re-locked here). + // After a StepPushed event, `activity.params` reflects the CURRENT step + // params (`makeActivityReducer.ts:78`). The original Pushed activityParams + // land in `steps[0].params`. Assert both are coerced strings. + expect(active?.steps[0]?.params.count).toEqual("42"); + expect(typeof active?.steps[0]?.params.count).toEqual("string"); + // stepParams branch — this is the new lock for T-I-NEW-5. + // `active.params` == last step's params after StepPushed. + expect(active?.params.offset).toEqual("7"); + expect(typeof active?.params.offset).toEqual("string"); + }); + + test("historySyncPlugin - FEP-1061: defaultHistory ancestor entries with typed activityParams + stepParams coerce (T-I-NEW-6)", async () => { + // T-I-NEW-6: `historyEntryToEvents` (historySyncPlugin.tsx:276-309) is + // invoked for `defaultHistory` ancestor entries. Boot via URL-arrival on + // a route whose `defaultHistory` returns an ancestor with TYPED + // `activityParams` and TYPED `stepParams`. Both must be coerced when + // the ancestor events are emitted. + const historyForDefault = createMemoryHistory({ + initialEntries: ["/articles/9/"], + }); + + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history: historyForDefault, + routes: { + Home: "/home/", + Article: { + path: "/articles/:articleId", + defaultHistory: () => [ + { + activityName: "Home", + // TYPED — should be coerced via historyEntryToEvents. + activityParams: { + count: 42 as unknown as string, + }, + additionalSteps: [ + { + stepParams: { + offset: 7 as unknown as string, + }, + }, + ], + }, + ], + }, + }, + fallbackActivity: () => "Home", + }), + ], + }); + + const proxyActions = makeActionsProxy({ actions: coreStore.actions }); + const stack = await proxyActions.getStack(); + + // The ancestor "Home" activity from defaultHistory. + const homeAncestor = stack.activities.find((a) => a.name === "Home"); + // After additionalSteps processing, `homeAncestor.params` reflects the + // LAST step's params (`makeActivityReducer.ts:78`). The original Pushed + // activityParams land in `steps[0].params`. + expect(homeAncestor?.steps[0]?.params.count).toEqual("42"); + expect(typeof homeAncestor?.steps[0]?.params.count).toEqual("string"); + // The step's stepParams are coerced and surfaced via homeAncestor.params + // (current-step alias) and also in the last step's params. + expect(homeAncestor?.params.offset).toEqual("7"); + expect(typeof homeAncestor?.params.offset).toEqual("string"); + + // The target Article activity — sanity-check it landed. + const article = stack.activities.find((a) => a.name === "Article"); + expect(article?.params.articleId).toEqual("9"); + }); + + test("historySyncPlugin - FEP-1061: popstate isStepBackward branch preserves coercion (T-I-NEW-9)", async () => { + // T-I-NEW-9: popstate `isStepBackward` (historySyncPlugin.tsx:538-554). + // When a back() navigates to a step that's no longer in the stack, the + // re-stepPush re-enters via `onBeforeStepPush` → coercion re-applies + // (idempotent on already-coerced strings, locks string-only on + // never-coerced typed entries). Asserts the round-trip property on a + // typed stepPush. + actions.push({ + activityId: "a1", + activityName: "Article", + activityParams: { + articleId: "1", + }, + }); + await actions.stepPush({ + stepId: "s1", + stepParams: { + articleId: "1", + visible: true as unknown as string, + count: 5 as unknown as string, + }, + }); + const beforeBack = await actions.getStack(); + const beforeActive = activeActivity(beforeBack); + const beforeStep = beforeActive?.steps[beforeActive.steps.length - 1]; + expect(typeof beforeStep?.params.visible).toEqual("string"); + expect(typeof beforeStep?.params.count).toEqual("string"); + + history.back(); + const afterBack = await actions.getStack(); + const afterActive = activeActivity(afterBack); + // The step has popped (back through the step). Verify all remaining + // step params on this activity are still string-only — regression lock. + for (const step of afterActive?.steps ?? []) { + for (const v of Object.values(step.params)) { + if (v !== undefined) { + expect(typeof v).toEqual("string"); + } + } + } + // Also the activity's params remain string-only. + for (const v of Object.values(afterActive?.params ?? {})) { + if (v !== undefined) { + expect(typeof v).toEqual("string"); + } + } + }); + + test("historySyncPlugin - FEP-1061: popstate isForward branch preserves coercion (T-I-NEW-10)", async () => { + // T-I-NEW-10: popstate `isForward` (historySyncPlugin.tsx:556-563). + // After back, forward must re-push the activity with params drawn from + // `targetActivity.params` (which were coerced when first pushed). Lock + // that the forward re-push preserves string-only. + await pushUntyped( + actions, + "Article", + { articleId: "1", visible: true, count: 7 }, + "a1", + ); + await actions.push({ + activityId: "a2", + activityName: "Article", + activityParams: { articleId: "2" }, + }); + + history.back(); + const backStack = await actions.getStack(); + expect(activeActivity(backStack)?.id).toEqual("a1"); + + history.forward(); + const fwdStack = await actions.getStack(); + const fwdActive = activeActivity(fwdStack); + // Active is the forward target (a2); a1's params remain string-only on + // the inactive entry, AND any re-push that happened did not introduce + // typed values. + for (const a of fwdStack.activities) { + for (const v of Object.values(a.params)) { + if (v !== undefined) { + expect(typeof v).toEqual("string"); + } + } + } + expect(fwdActive?.id).toEqual("a2"); + }); + + test("historySyncPlugin - FEP-1061: popstate isStepForward branch preserves coercion (T-I-NEW-11)", async () => { + // T-I-NEW-11: popstate `isStepForward` (historySyncPlugin.tsx:564-574). + // stepPush typed → back → forward. The forward stepPush draws from + // `targetStep.params`. Asserts the entire stack's step params remain + // string-only after the round-trip. + actions.push({ + activityId: "a1", + activityName: "Article", + activityParams: { articleId: "1" }, + }); + await actions.stepPush({ + stepId: "s1", + stepParams: { + articleId: "1", + visible: true as unknown as string, + count: 7 as unknown as string, + }, + }); + + history.back(); + await actions.getStack(); + history.forward(); + + const fwdStack = await actions.getStack(); + for (const a of fwdStack.activities) { + for (const v of Object.values(a.params)) { + if (v !== undefined) { + expect(typeof v).toEqual("string"); + } + } + for (const step of a.steps) { + for (const v of Object.values(step.params)) { + if (v !== undefined) { + expect(typeof v).toEqual("string"); + } + } + } + } + }); + + // T-I-NEW-12: mapInitialActivityPlugin × historySyncPlugin ordering. + // SKIP RATIONALE: `@stackflow/plugin-map-initial-activity` is NOT a + // devDependency of `@stackflow/plugin-history-sync` (verified via + // `extensions/plugin-history-sync/package.json` — no entry). Adding it + // would require introducing a new dependency, which is out-of-scope per + // the executor task's constraints. The plugin source DOES exist + // (`extensions/plugin-map-initial-activity/src/mapInitialActivityPlugin.tsx`), + // and the cross-plugin interaction analysis classifies the + // ordering as a `medium` FEP-1061 risk (gap), so the *theoretical* test + // surface is documented here for a future maintainer who can add the + // dep. The plugin uses `window.location.href` directly (line 20), making + // it additionally awkward to drive from a `MemoryHistory` test — a + // realistic test would need to stub `window.location` or use jsdom. + test.skip("historySyncPlugin - FEP-1061: mapInitialActivityPlugin × history-sync overrideInitialEvents ordering (T-I-NEW-12, see comment)", () => { + // Documented limitation per the cross-plugin interaction analysis: + // - When mapInitialActivityPlugin is registered AFTER historySyncPlugin + // in the plugins array, its `overrideInitialEvents` runs SECOND and + // replaces the entire event array with a single Pushed event whose + // `activityParams` came from `options.mapper(URL)` — typed values + // from the mapper SURVIVE uncoerced (Risk #6-pattern at + // overrideInitialEvents boundary). + // - When registered BEFORE historySyncPlugin, history-sync's + // `overrideInitialEvents` ignores upstream events (it's not a + // fold-over-events plugin — it consults `history.location` and + // defaultHistory) and replaces them, so the mapper's typed values + // are dropped and history-sync coercion applies to URL-derived + // params. + // Source: extensions/plugin-map-initial-activity/src/mapInitialActivityPlugin.tsx + expect(true).toBe(true); + }); + + // T-I-NEW-13: preloadPlugin × historySyncPlugin spread re-emission. + // SKIP RATIONALE: `@stackflow/plugin-preload` is NOT a devDependency of + // `@stackflow/plugin-history-sync` (verified via package.json). Adding + // it would require introducing a new dependency, which is out-of-scope. + // The plugin source DOES exist + // (`extensions/plugin-preload/src/pluginPreload.tsx`); search-3 classifies + // this combination as `medium` risk (gap). The Risk #6 spread-re-emission + // pattern IS already locked by the existing + // `historySyncPlugin - FEP-1061: Risk #6` test (line ~1742) which uses a + // hand-rolled `laterPlugin` mirroring preloadPlugin's `overrideActionParams({...actionParams, activityContext: {...}})` shape (see + // pluginPreload.tsx:81-87). The semantic test surface is therefore + // already covered transitively; only the literal "use the real + // preloadPlugin" assertion is gated on the dep. + test.skip("historySyncPlugin - FEP-1061: real preloadPlugin × history-sync spread-re-emission (T-I-NEW-13, see comment)", () => { + // Theoretical assertion (when dep added): register + // `[historySyncPlugin, preloadPlugin]` (preload AFTER); push with typed + // boolean param. preloadPlugin's `onBeforePush` calls + // `overrideActionParams({ ...actionParams, activityContext: { ..., preloadRef } })` + // (pluginPreload.tsx:81-87). Because the spread re-emits the + // already-coerced `activityParams` from the prior history-sync + // `onBeforePush`, the store ends with `visible === "true"` (string). + // This is distinct from the existing Risk #6 hand-rolled test in that + // preloadPlugin spreads activityParams unchanged (safe), whereas the + // hand-rolled test re-asserts a TYPED value (clobbers coercion). + expect(true).toBe(true); + }); + + describe.skip("FEP-1061 — Linear ticket interpretation #1 — type widening (NOT chosen, see INTENT.md)", () => { + // These assertions PASS only under interpretation #1 (widen + // ActivityBaseParams to `unknown`). They are intentionally skipped + // because the implementation chose interpretation #3 (the originating user + // quote: always-string at plugin boundary). See: + // - extensions/plugin-history-sync/INTENT.md + // - the project's internal tracker reference + // - https://github.com/daangn/stackflow/pull/705 + // + // To unskip these, ActivityBaseParams must be widened in + // @stackflow/config and coerceParamsToString deleted from this plugin. + // The assertions below describe exactly what would have to change. + + test("interpretation #1: push({ visible: true }) preserves boolean in store", async () => { + await pushUntyped(actions, "Article", { + articleId: "1", + visible: true, + }); + const stack = await actions.getStack(); + const active = activeActivity(stack); + // Would PASS only under interpretation #1 (no coercion). + expect(typeof active?.params.visible).toEqual("boolean"); + expect(active?.params.visible).toEqual(true); + }); + + test("interpretation #1: useActivityParams returns numeric count from typed decode", async () => { + const ssrHistory = createMemoryHistory({ + initialEntries: ["/articles/1/?count=5"], + }); + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history: ssrHistory, + routes: { + Home: "/home/", + Article: { + path: "/articles/:articleId", + decode: (p) => ({ + articleId: p.articleId, + count: Number(p.count) as unknown as string, + }), + }, + }, + fallbackActivity: () => "Home", + }), + ], + }); + const proxyActions = makeActionsProxy({ actions: coreStore.actions }); + const stack = await proxyActions.getStack(); + const active = activeActivity(stack); + // Would PASS only under interpretation #1 (no coercion at plugin boundary). + expect(typeof active?.params.count).toEqual("number"); + expect(active?.params.count).toEqual(5); + }); + }); + + // ────────────────────────────────────────────────────────────────────── + // FEP-1061 — Phase A RED tests (PR review v2.1) + // + // These tests assert the URL surface (path(history.location)) — not just + // the store surface. They prove Issue #1 (encode-output not written to + // history) and Issue #2 (popstate forward re-pushing with coerced strings). + // ────────────────────────────────────────────────────────────────────── + + test("historySyncPlugin - FEP-1061: T-O-1 push with non-identity encode → history.location reflects encode output", async () => { + history = createMemoryHistory(); + + const encode = (params: Record) => ({ + articleId: String(params.articleId), + visible: params.visible ? "y" : "n", + }); + + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: "/home/", + Article: { + path: "/articles/:articleId", + encode, + }, + }, + fallbackActivity: () => "Home", + }), + ], + }); + const a = makeActionsProxy({ actions: coreStore.actions }); + + await a.push({ + activityId: "a1", + activityName: "Article", + activityParams: { + articleId: "1234", + visible: true as unknown as string, + }, + }); + + expect(path(history.location)).toEqual("/articles/1234/?visible=y"); + }); + + test("historySyncPlugin - FEP-1061: T-O-2 replace with non-identity encode → history.location reflects encode output", async () => { + history = createMemoryHistory(); + + const encode = (params: Record) => ({ + articleId: String(params.articleId), + visible: params.visible ? "y" : "n", + }); + + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: "/home/", + Article: { + path: "/articles/:articleId", + encode, + }, + }, + fallbackActivity: () => "Home", + }), + ], + }); + const a = makeActionsProxy({ actions: coreStore.actions }); + + await a.replace({ + activityId: "a1", + activityName: "Article", + activityParams: { + articleId: "1234", + visible: true as unknown as string, + }, + }); + + expect(path(history.location)).toEqual("/articles/1234/?visible=y"); + }); + + test("historySyncPlugin - FEP-1061: T-O-3 stepPush with non-identity encode → history.location reflects encode output", async () => { + history = createMemoryHistory(); + + const encode = (params: Record) => ({ + articleId: String(params.articleId), + visible: params.visible ? "y" : "n", + }); + + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: "/home/", + Article: { + path: "/articles/:articleId", + encode, + }, + }, + fallbackActivity: () => "Home", + }), + ], + }); + const a = makeActionsProxy({ actions: coreStore.actions }); + + await a.push({ + activityId: "a1", + activityName: "Article", + activityParams: { + articleId: "1", + visible: false as unknown as string, + }, + }); + await a.stepPush({ + stepId: "s1", + stepParams: { + articleId: "2", + visible: true as unknown as string, + }, + }); + + expect(path(history.location)).toEqual("/articles/2/?visible=y"); + }); + + test("historySyncPlugin - FEP-1061: T-O-4 stepReplace with non-identity encode → history.location reflects encode output", async () => { + history = createMemoryHistory(); + + const encode = (params: Record) => ({ + articleId: String(params.articleId), + visible: params.visible ? "y" : "n", + }); + + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: "/home/", + Article: { + path: "/articles/:articleId", + encode, + }, + }, + fallbackActivity: () => "Home", + }), + ], + }); + const a = makeActionsProxy({ actions: coreStore.actions }); + + await a.push({ + activityId: "a1", + activityName: "Article", + activityParams: { + articleId: "1", + visible: false as unknown as string, + }, + }); + await a.stepPush({ + stepId: "s1", + stepParams: { + articleId: "2", + visible: false as unknown as string, + }, + }); + await a.stepReplace({ + stepId: "s2", + stepParams: { + articleId: "3", + visible: true as unknown as string, + }, + }); + + expect(path(history.location)).toEqual("/articles/3/?visible=y"); + }); + + test("historySyncPlugin - FEP-1061: T-O-5 defaultHistory ancestor URL uses ancestor's route encode (not currentPath)", async () => { + // Arrive on Article URL with a typed-decode chain; the defaultHistory + // declares Home as the ancestor. The ancestor URL pushed in + // historyEntryToEvents should reflect Home's route encode (or its plain + // template), NOT the current Article path. + history = createMemoryHistory({ + initialEntries: ["/articles/9/?visible=true"], + }); + + const homeEncode = jest.fn((p: Record) => ({ + articleId: String(p.articleId ?? ""), + visible: p.visible ? "y" : "n", + })); + + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: { + path: "/home/", + encode: homeEncode, + }, + Article: { + path: "/articles/:articleId", + defaultHistory: () => [ + { + activityName: "Home", + activityParams: { + visible: true as unknown as string, + }, + }, + ], + }, + } as any, + fallbackActivity: () => "Home", + }), + ], + }); + const a = makeActionsProxy({ actions: coreStore.actions }); + // Allow defaultHistory replay (Home ancestor → Article target) to settle + // through onChanged → push/stepPush. + await a.getStack(); + await a.getStack(); + await a.getStack(); + + // Walk back to the Home ancestor entry to inspect its URL. + history.back(); + await a.getStack(); + + // Ancestor URL pushed during defaultHistory replay must use Home's encode + // output (visible=y), NOT the Article path the user arrived on. + expect(path(history.location)).toEqual("/home/?visible=y"); + }); + + test("historySyncPlugin - FEP-1061: T-O-6 popstate forward (activity boundary): encode receives typed-via-context, NOT coerced strings", async () => { + history = createMemoryHistory(); + + const encode = jest.fn((params: Record) => ({ + articleId: String(params.articleId), + visible: params.visible ? "y" : "n", + })); + + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: "/home/", + Article: { + path: "/articles/:articleId", + encode, + }, + }, + fallbackActivity: () => "Home", + }), + ], + }); + const a = makeActionsProxy({ actions: coreStore.actions }); + + await a.push({ + activityId: "a1", + activityName: "Article", + activityParams: { + articleId: "1234", + visible: true as unknown as string, + }, + }); + + // Snapshot encode call list to detect post-popstate-forward calls. + const callsAfterPush = encode.mock.calls.length; + + history.back(); + await a.getStack(); + history.forward(); + await a.getStack(); + + // If encode was called again on the popstate-forward branch (Issue #2), + // it would have received the coerced-string `visible: "true"` (truthy → + // "y") and produced /articles/1234/?visible=y — by accident. The robust + // assertion: encode mock should NOT see `typeof === "string"` for + // `visible` after the push (only typed boolean). + const allCalls = encode.mock.calls.slice(callsAfterPush); + for (const call of allCalls) { + const arg = call[0] as Record; + if ("visible" in arg) { + expect(typeof arg.visible).not.toEqual("string"); + } + } + + // Final URL should be encode-output (not coerced). + expect(path(history.location)).toEqual("/articles/1234/?visible=y"); + }); + + test("historySyncPlugin - FEP-1061: T-O-7 popstate stepForward: encode-output URL preserved through step.context.path", async () => { + history = createMemoryHistory(); + + const encode = (params: Record) => ({ + articleId: String(params.articleId), + visible: params.visible ? "y" : "n", + }); + + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: "/home/", + Article: { + path: "/articles/:articleId", + encode, + }, + }, + fallbackActivity: () => "Home", + }), + ], + }); + const a = makeActionsProxy({ actions: coreStore.actions }); + + await a.push({ + activityId: "a1", + activityName: "Article", + activityParams: { + articleId: "1", + visible: false as unknown as string, + }, + }); + await a.stepPush({ + stepId: "s1", + stepParams: { + articleId: "2", + visible: true as unknown as string, + }, + }); + + const expectedStepUrl = "/articles/2/?visible=y"; + expect(path(history.location)).toEqual(expectedStepUrl); + + // back to activity, forward to step + history.back(); + await a.getStack(); + history.forward(); + await a.getStack(); + + // step URL must be preserved through step.context.path (Option B): + expect(path(history.location)).toEqual(expectedStepUrl); + }); + + test("historySyncPlugin - FEP-1061: T-O-8 onInit URL-replay reflects encode output for parsed-state restoration", async () => { + // Boot with initialEntries containing a serialized state whose + // activity.context.path is the encode-output URL (e.g. saved by an + // earlier deploy / SSR). After onInit, history.location should reflect + // that encoded URL, NOT a fillWithoutEncode-derived URL. + const { stringify: flattedStringify } = await import("flatted"); + const STATE_TAG = "@stackflow/plugin-history-sync"; + const serializedState = { + _TAG: STATE_TAG, + flattedState: flattedStringify({ + activity: { + id: "a1", + name: "Article", + transitionState: "enter-done", + params: { articleId: "1234", visible: "true" }, + context: { path: "/articles/1234/?visible=y" }, + steps: [], + enteredBy: { + id: "evt-1", + eventDate: 1, + name: "Pushed", + activityId: "a1", + activityName: "Article", + activityParams: { articleId: "1234", visible: "true" }, + activityContext: { path: "/articles/1234/?visible=y" }, + }, + isTop: true, + isActive: true, + isRoot: true, + zIndex: 0, + }, + step: undefined, + }), + }; + history = createMemoryHistory({ + initialEntries: [ + { + pathname: "/articles/1234/?visible=y", + state: serializedState, + }, + ], + }); + + const encode = (params: Record) => ({ + articleId: String(params.articleId), + visible: params.visible ? "y" : "n", + }); + + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: "/home/", + Article: { + path: "/articles/:articleId", + encode, + }, + }, + fallbackActivity: () => "Home", + }), + ], + }); + const a = makeActionsProxy({ actions: coreStore.actions }); + await a.getStack(); + + // onInit's parsed-state branch should not overwrite history with a + // fillWithoutEncode URL — the trusted state already contains the + // encode-output path. + expect(path(history.location)).toEqual("/articles/1234/?visible=y"); + }); + + test("historySyncPlugin - FEP-1061: T-O-12 route WITHOUT encode → history.location byte-identical to fillWithoutEncode output (regression bar)", async () => { + // Regression bar: must PASS even on the unfixed branch. Routes without + // encode should write URLs identical to fillWithoutEncode of coerced + // params. + history = createMemoryHistory(); + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: "/home/", + Article: "/articles/:articleId", + }, + fallbackActivity: () => "Home", + }), + ], + }); + const a = makeActionsProxy({ actions: coreStore.actions }); + + await a.push({ + activityId: "a1", + activityName: "Article", + activityParams: { + articleId: "1234", + visible: true as unknown as string, + }, + }); + + // No encode → URL contains the coerced string "true" (same as main). + expect(path(history.location)).toEqual("/articles/1234/?visible=true"); + }); + + test("historySyncPlugin - FEP-1061: T-O-14 SSR — server-emitted activity.context.path is trusted on client onInit replay", async () => { + // Simulate a server that already computed an encoded URL using encode + // and put it into activity.context.path. Cross-deploy hydration uses + // history.state to carry server-side activity context. The client + // onInit's parsed-state branch should preserve that URL rather than + // recompute it via fillWithoutEncode. + const { stringify: flattedStringify } = await import("flatted"); + const STATE_TAG = "@stackflow/plugin-history-sync"; + const serverEncodedUrl = "/articles/1234/?visible=y"; + const serializedState = { + _TAG: STATE_TAG, + flattedState: flattedStringify({ + activity: { + id: "a-ssr", + name: "Article", + transitionState: "enter-done", + // server-coerced strings (FEP-1061 contract) + params: { articleId: "1234", visible: "true" }, + // server-emitted encode-output URL + context: { path: serverEncodedUrl }, + steps: [], + enteredBy: { + id: "evt-ssr-1", + eventDate: 1, + name: "Pushed", + activityId: "a-ssr", + activityName: "Article", + activityParams: { articleId: "1234", visible: "true" }, + activityContext: { path: serverEncodedUrl }, + }, + isTop: true, + isActive: true, + isRoot: true, + zIndex: 0, + }, + step: undefined, + }), + }; + // boot with the server-emitted URL and state + history = createMemoryHistory({ + initialEntries: [ + { + pathname: serverEncodedUrl, + state: serializedState, + }, + ], + }); + + const encode = (params: Record) => ({ + articleId: String(params.articleId), + visible: params.visible ? "y" : "n", + }); + + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: "/home/", + Article: { + path: "/articles/:articleId", + encode, + }, + }, + fallbackActivity: () => "Home", + }), + ], + }); + const a = makeActionsProxy({ actions: coreStore.actions }); + await a.getStack(); + + // The parsed-state path is trusted; URL is preserved as the server + // wrote it (encode output), not recomputed via fillWithoutEncode. + expect(path(history.location)).toEqual(serverEncodedUrl); + }); + + test("historySyncPlugin - FEP-1061: T-O-16 replace() of activity with 3 active steps → no orphan; new activity URL is encode-output-correct", async () => { + history = createMemoryHistory(); + + const encode = (params: Record) => ({ + articleId: String(params.articleId ?? ""), + thirdId: String(params.thirdId ?? ""), + visible: params.visible ? "y" : "n", + }); + + const coreStore = stackflow({ + activityNames: ["Home", "Article", "ThirdActivity"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: "/home/", + Article: "/articles/:articleId", + ThirdActivity: { + path: "/third/:thirdId", + encode, + }, + }, + fallbackActivity: () => "Home", + }), + ], + }); + const a = makeActionsProxy({ actions: coreStore.actions }); + + await a.push({ + activityId: "a1", + activityName: "Article", + activityParams: { articleId: "1" }, + }); + await a.stepPush({ stepId: "s1", stepParams: { articleId: "1a" } }); + await a.stepPush({ stepId: "s2", stepParams: { articleId: "1b" } }); + await a.stepPush({ stepId: "s3", stepParams: { articleId: "1c" } }); + + await a.replace({ + activityId: "a2", + activityName: "ThirdActivity", + activityParams: { + thirdId: "9", + visible: true as unknown as string, + }, + }); + + const stack = await a.getStack(); + const active = activeActivity(stack); + expect(active?.name).toEqual("ThirdActivity"); + // No orphan steps from the replaced activity. + expect(active?.steps.length).toBeLessThanOrEqual(1); + // New URL reflects new route's encode output. + expect(path(history.location)).toEqual("/third/9/?visible=y"); + }); + + // ────────────────────────────────────────────────────────────────────── + // FEP-1061 — current-behavior pins (not Phase A RED) + // ────────────────────────────────────────────────────────────────────── + + describe("FEP-1061 — current-behavior pins (not Phase A RED)", () => { + test("T-O-13 plugin dispatches Pushed without activityContext → post-effect falls back to fillWithoutEncode", async () => { + // A plugin registered AFTER history-sync that re-emits Pushed without + // an activityContext should cause the post-effect to fall back to + // fillWithoutEncode (no path crash; URL uses coerced params). This + // documents the S1 fallback. + history = createMemoryHistory(); + + const stripContextPlugin: StackflowPlugin = () => ({ + key: "strip-context", + onBeforePush({ actionParams, actions: { overrideActionParams } }) { + // Strip activityContext set by upstream history-sync to simulate + // a plugin that doesn't carry the path forward. + const { activityContext, ...rest } = actionParams as Record< + string, + unknown + >; + overrideActionParams(rest as typeof actionParams); + }, + }); + + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: "/home/", + Article: "/articles/:articleId", + }, + fallbackActivity: () => "Home", + }), + stripContextPlugin, + ], + }); + const a = makeActionsProxy({ actions: coreStore.actions }); + + await a.push({ + activityId: "a1", + activityName: "Article", + activityParams: { articleId: "1234", visible: "true" }, + }); + + // Fallback to fillWithoutEncode — URL still produced (coerced strings). + expect(path(history.location)).toEqual("/articles/1234/?visible=true"); + }); + + test("T-O-15 plugin module has no closure-captured Map state (HMR safety)", async () => { + // Re-instantiate the plugin twice; each instance should be fully + // independent (no shared module state). Verifies Option B preserves + // SSoT — no parallel Map state inside the plugin closure. + const h1 = createMemoryHistory(); + const h2 = createMemoryHistory(); + + const factory = () => + historySyncPlugin({ + history: h1, + routes: { + Home: "/home/", + Article: "/articles/:articleId", + }, + fallbackActivity: () => "Home", + }); + + const plugin1 = factory(); + const plugin2 = historySyncPlugin({ + history: h2, + routes: { + Home: "/home/", + Article: "/articles/:articleId", + }, + fallbackActivity: () => "Home", + }); + + // Both factory results should be independent functions producing + // independent plugin instances. + const inst1 = plugin1(); + const inst2 = plugin2(); + + expect(inst1).not.toBe(inst2); + expect(inst1.key).toEqual("plugin-history-sync"); + expect(inst2.key).toEqual("plugin-history-sync"); + + // Enumerable closure variables on the plugin factory return object + // should be limited to the lifecycle hooks + key + wrapStack — + // i.e. NO state Map keys leaking out. (We snapshot the keys to + // detect a regression that adds module-level Map state.) + const inst1Keys = Object.keys(inst1).sort(); + const inst2Keys = Object.keys(inst2).sort(); + expect(inst1Keys).toStrictEqual(inst2Keys); + }); + }); + + // ────────────────────────────────────────────────────────────────────── + // FEP-1061 — Phase A strengthening of existing tests (T-O-10, T-O-11) + // ────────────────────────────────────────────────────────────────────── + + test("historySyncPlugin - FEP-1061: T-O-10 STRENGTHEN T-I1 — encode-ORDER LOCK also asserts path(history.location)", async () => { + // Strengthens the existing T-I1 (line ~2037) by adding the URL surface + // assertion: path(history.location) must equal the encode-output URL, + // not the fillWithoutEncode URL. + history = createMemoryHistory(); + + const encode = (params: Record) => ({ + articleId: String(params.articleId), + visible: params.visible ? "y" : "n", + }); + + const coreStore = stackflow({ + activityNames: ["Home", "Article"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: "/home", + Article: { + path: "/articles/:articleId", + encode, + }, + }, + fallbackActivity: () => "Home", + }), + ], + }); + const a = makeActionsProxy({ actions: coreStore.actions }); + + await a.push({ + activityId: "a1", + activityName: "Article", + activityParams: { + articleId: "1234", + visible: true as unknown as string, + }, + }); + + expect(path(history.location)).toEqual("/articles/1234/?visible=y"); + }); + + test("historySyncPlugin - FEP-1061: T-O-11 STRENGTHEN F3 — replace-route atomicity also asserts path(history.location) reflects new route's encode-output", async () => { + // Strengthens the existing F3 (line ~2573). The third route uses an + // identity encode (or no encode), so the URL == the path; but verify + // the URL matches the new route's encode-output. + history = createMemoryHistory(); + + const coreStore = stackflow({ + activityNames: ["Home", "Article", "ThirdActivity"], + plugins: [ + historySyncPlugin({ + history, + routes: { + Home: "/home/", + Article: "/articles/:articleId", + ThirdActivity: "/third/:thirdId", + }, + fallbackActivity: () => "Home", + }), + ], + }); + const a = makeActionsProxy({ actions: coreStore.actions }); + + await a.push({ + activityId: "a1", + activityName: "Article", + activityParams: { articleId: "1" }, + }); + + await a.replace({ + activityId: "a2", + activityName: "ThirdActivity", + activityParams: { + thirdId: "9", + visible: true as unknown as string, + }, + }); + + expect(path(history.location)).toEqual("/third/9/?visible=true"); + }); }); diff --git a/extensions/plugin-history-sync/src/historySyncPlugin.tsx b/extensions/plugin-history-sync/src/historySyncPlugin.tsx index 02164e725..da1287806 100644 --- a/extensions/plugin-history-sync/src/historySyncPlugin.tsx +++ b/extensions/plugin-history-sync/src/historySyncPlugin.tsx @@ -18,6 +18,7 @@ import UrlPattern from "url-pattern"; import { ActivityActivationCountsContext } from "./ActivityActivationCountsContext"; import type { ActivityActivationMonitor } from "./ActivityActivationMonitor/ActivityActivationMonitor"; import { DefaultHistoryActivityActivationMonitor } from "./ActivityActivationMonitor/DefaultHistoryActivityActivationMonitor"; +import { coerceParamsToString } from "./coerceParamsToString"; import { HistoryQueueProvider } from "./HistoryQueueContext"; import { parseState, pushState, replaceState } from "./historyState"; import { last } from "./last"; @@ -195,10 +196,19 @@ export function historySyncPlugin< const initialState = parseState(history.location.state); if (initialState) { + // FEP-1061: cross-deploy hydration. `initialState` was serialized by + // some earlier plugin version (possibly pre-FEP-1061) and may carry + // typed values in `activityParams` / `stepParams`. Coerce here so the + // 7th entry path (parseState early-return) also enforces the + // string-only invariant. Within-deploy this is idempotent — the + // writer already coerced. return [ { ...initialState.activity.enteredBy, name: "Pushed", + activityParams: coerceParamsToString( + initialState.activity.enteredBy.activityParams, + ), }, ...(initialState.step?.enteredBy.name === "StepPushed" || initialState.step?.enteredBy.name === "StepReplaced" @@ -206,6 +216,9 @@ export function historySyncPlugin< { ...initialState.step.enteredBy, name: "StepPushed" as const, + stepParams: coerceParamsToString( + initialState.step.enteredBy.stepParams, + ), }, ] : []), @@ -271,54 +284,88 @@ export function historySyncPlugin< }: HistoryEntry): ( | Omit | Omit - )[] => [ - { + )[] => { + // FEP-1061 (Option B, B8): per-ancestor URL via the ancestor's + // own route encode (was previously `currentPath`, which leaked + // the URL the user arrived on into all ancestor entries). + const ancestorRoute = activityRoutes.find( + (r) => r.activityName === activityName, + ); + const ancestorTemplate = ancestorRoute + ? makeTemplate(ancestorRoute, options.urlPatternOptions) + : null; + const ancestorActivityPath = ancestorTemplate + ? ancestorTemplate.fill(activityParams) + : currentPath; + + return [ + { + name: "Pushed", + id: id(), + activityId: id(), + activityName, + activityParams: coerceParamsToString(activityParams), + activityContext: { + path: ancestorActivityPath, + lazyActivityComponentRenderContext: { + shouldRenderImmediately: true, + }, + }, + }, + ...additionalSteps.map( + ({ + stepParams, + hasZIndex, + }): Omit => ({ + name: "StepPushed", + id: id(), + stepId: id(), + stepParams: coerceParamsToString(stepParams), + ...(ancestorTemplate + ? { + stepContext: { + path: ancestorTemplate.fill(stepParams), + }, + } + : {}), + hasZIndex, + }), + ), + ]; + }; + const createTargetActivityPushEvent = (): Omit< + PushedEvent, + "eventDate" + > => { + const targetTemplate = makeTemplate( + targetActivityRoute, + options.urlPatternOptions, + ); + const matched = targetTemplate.parse(currentPath); + const targetParams = + matched ?? urlSearchParamsToMap(pathToUrl(currentPath).searchParams); + // FEP-1061 (Option B, B8): when the URL matched the target route, + // use currentPath (the user's URL); when fallback was triggered + // (no match), compute the target route's URL from its template + // so onInit writes a route-correct URL (was previously + // currentPath, e.g. "/" instead of "/home/"). + const targetPath = matched + ? currentPath + : targetTemplate.fill(targetParams); + return { name: "Pushed", id: id(), activityId: id(), - activityName, - activityParams: { - ...activityParams, - }, + activityName: targetActivityRoute.activityName, + activityParams: coerceParamsToString(targetParams), activityContext: { - path: currentPath, + path: targetPath, lazyActivityComponentRenderContext: { shouldRenderImmediately: true, }, }, - }, - ...additionalSteps.map( - ({ - stepParams, - hasZIndex, - }): Omit => ({ - name: "StepPushed", - id: id(), - stepId: id(), - stepParams, - hasZIndex, - }), - ), - ]; - const createTargetActivityPushEvent = (): Omit< - PushedEvent, - "eventDate" - > => ({ - name: "Pushed", - id: id(), - activityId: id(), - activityName: targetActivityRoute.activityName, - activityParams: - makeTemplate(targetActivityRoute, options.urlPatternOptions).parse( - currentPath, - ) ?? urlSearchParamsToMap(pathToUrl(currentPath).searchParams), - activityContext: { - path: currentPath, - lazyActivityComponentRenderContext: { - shouldRenderImmediately: true, - }, - }, - }); + }; + }; if (defaultHistory.skipDefaultHistorySetupTransition) { initialSetupProcess = new SerialNavigationProcess([ @@ -402,10 +449,17 @@ export function historySyncPlugin< )!; const template = makeTemplate(match, options.urlPatternOptions); + // FEP-1061 (Option B): trust activity.context.path (computed by + // onBeforePush from typed params before coercion, or set by SSR) + // and fall back to fillWithoutEncode only when missing. + const activityPath = + (activity.context as { path?: string } | undefined)?.path ?? + template.fillWithoutEncode(activity.params); + if (activity.isRoot) { replaceState({ history, - pathname: template.fill(activity.params), + pathname: activityPath, state: { activity: activity, }, @@ -414,7 +468,7 @@ export function historySyncPlugin< } else { pushState({ history, - pathname: template.fill(activity.params), + pathname: activityPath, state: { activity: activity, }, @@ -424,9 +478,14 @@ export function historySyncPlugin< for (const step of activity.steps) { if (!step.exitedBy && step.enteredBy.name !== "Pushed") { + // FEP-1061 (Option B): trust step.context.path (set by + // onBeforeStepPush from typed params), fall back otherwise. + const stepPath = + (step.context as { path?: string } | undefined)?.path ?? + template.fillWithoutEncode(step.params); pushState({ history, - pathname: template.fill(step.params), + pathname: stepPath, state: { activity: activity, step: step, @@ -551,6 +610,11 @@ export function historySyncPlugin< activityId: targetActivity.id, activityName: targetActivity.name, activityParams: targetActivity.params, + // FEP-1061 (Option B, B4): preserve the encoded path through + // popstate-forward re-push so onBeforePush's "skip when path + // already present" branch fires and encode is NOT re-run on + // the coerced strings. + activityContext: targetActivity.context, }); } if (isStepForward()) { @@ -562,6 +626,9 @@ export function historySyncPlugin< stepPush({ stepId: targetStep.id, stepParams: targetStep.params, + // FEP-1061 (Option B, B7): preserve the encoded step path + // through popstate-stepForward re-push. + stepContext: targetStep.context, }); } }; @@ -588,11 +655,20 @@ export function historySyncPlugin< const template = makeTemplate(match, options.urlPatternOptions); + // FEP-1061 (Option B, B3): trust activity.context.path written by + // onBeforePush (which ran encode on the typed params before + // coercion). Fall back to fillWithoutEncode only when missing + // (e.g. plugin re-emits Pushed without activityContext — see + // T-O-13 pin). + const pathname = + (activity.context as { path?: string } | undefined)?.path ?? + template.fillWithoutEncode(activity.params); + requestHistoryTick(() => { silentFlag = true; pushState({ history, - pathname: template.fill(activity.params), + pathname, state: { activity, }, @@ -612,11 +688,20 @@ export function historySyncPlugin< const template = makeTemplate(match, options.urlPatternOptions); + // FEP-1061 (Option B, B6): trust step.context.path written by + // onBeforeStepPush. Fall back to fillWithoutEncode(activity.params) + // when missing — note this matches the pre-fix shape, which used + // activity.params (the latest set, often equal to step params), + // preserving behavior on the fallback path. + const pathname = + (step.context as { path?: string } | undefined)?.path ?? + template.fillWithoutEncode(activity.params); + requestHistoryTick(() => { silentFlag = true; pushState({ history, - pathname: template.fill(activity.params), + pathname, state: { activity, step, @@ -636,11 +721,16 @@ export function historySyncPlugin< const template = makeTemplate(match, options.urlPatternOptions); + // FEP-1061 (Option B, B3): see onPushed — same pattern. + const pathname = + (activity.context as { path?: string } | undefined)?.path ?? + template.fillWithoutEncode(activity.params); + requestHistoryTick(() => { silentFlag = true; replaceState({ history, - pathname: template.fill(activity.params), + pathname, state: { activity, }, @@ -659,11 +749,16 @@ export function historySyncPlugin< const template = makeTemplate(match, options.urlPatternOptions); + // FEP-1061 (Option B, B6): see onStepPushed — same pattern. + const pathname = + (step.context as { path?: string } | undefined)?.path ?? + template.fillWithoutEncode(activity.params); + requestHistoryTick(() => { silentFlag = true; replaceState({ history, - pathname: template.fill(activity.params), + pathname, state: { activity, step, @@ -673,48 +768,72 @@ export function historySyncPlugin< }); }, onBeforePush({ actionParams, actions: { overrideActionParams } }) { - if ( + const needsPath = !actionParams.activityContext || - "path" in actionParams.activityContext === false - ) { + "path" in actionParams.activityContext === false; + + // `template.fill` runs `encode` on the typed params U. We must call + // it BEFORE coercing so `encode` sees the original typed values + // (FEP-1061 contract). + let path: string | undefined; + if (needsPath) { const match = activityRoutes.find( (r) => r.activityName === actionParams.activityName, )!; const template = makeTemplate(match, options.urlPatternOptions); - const path = template.fill(actionParams.activityParams); - - overrideActionParams({ - ...actionParams, - activityContext: { - ...actionParams.activityContext, - path, - }, - }); + path = template.fill(actionParams.activityParams); } + + // FEP-1061: single `overrideActionParams` call so the path set above + // survives alongside the coerced `activityParams`. `core`'s + // `overrideActionParams` is a spread-merge, so splitting into two + // calls where the second spreads the ORIGINAL `actionParams` would + // clobber the just-set `activityContext.path`. + overrideActionParams({ + ...actionParams, + ...(needsPath + ? { + activityContext: { + ...actionParams.activityContext, + path, + }, + } + : {}), + activityParams: coerceParamsToString(actionParams.activityParams), + }); }, onBeforeReplace({ actionParams, actions: { overrideActionParams, getStack }, }) { - if ( + const needsPath = !actionParams.activityContext || - "path" in actionParams.activityContext === false - ) { + "path" in actionParams.activityContext === false; + + // See `onBeforePush` — `encode` must run on typed params first, and + // the single-call shape preserves path alongside coerced params. + let path: string | undefined; + if (needsPath) { const match = activityRoutes.find( (r) => r.activityName === actionParams.activityName, )!; const template = makeTemplate(match, options.urlPatternOptions); - const path = template.fill(actionParams.activityParams); - - overrideActionParams({ - ...actionParams, - activityContext: { - ...actionParams.activityContext, - path, - }, - }); + path = template.fill(actionParams.activityParams); } + overrideActionParams({ + ...actionParams, + ...(needsPath + ? { + activityContext: { + ...actionParams.activityContext, + path, + }, + } + : {}), + activityParams: coerceParamsToString(actionParams.activityParams), + }); + const { activities } = getStack(); const enteredActivities = activities.filter( (currentActivity) => @@ -744,6 +863,96 @@ export function historySyncPlugin< } } }, + onBeforeStepPush({ + actionParams, + actions: { getStack, overrideActionParams }, + }) { + // FEP-1061 (Option B, B5): if the caller already supplied a + // stepContext.path (e.g. popstate stepForward branch), preserve it. + // Otherwise compute it via the active activity's route template + // running encode on the TYPED params before coercion. If no active + // activity exists at dispatch time (initial boot / cross-deploy + // hydration before the parent Pushed materializes), gracefully + // skip path computation — the post-effect will fall back to + // fillWithoutEncode (Architect N3/N4). + const ctx = actionParams.stepContext as + | { path?: string } + | undefined; + const hasExistingPath = !!ctx && "path" in ctx; + + let path: string | undefined; + if (hasExistingPath) { + path = ctx?.path; + } else { + const stack = getStack(); + const activeActivity = stack.activities.find((a) => a.isActive); + if (activeActivity) { + const match = activityRoutes.find( + (r) => r.activityName === activeActivity.name, + ); + if (match) { + const template = makeTemplate(match, options.urlPatternOptions); + path = template.fill(actionParams.stepParams); + } + } + // else: no active activity (initial boot / cross-deploy + // hydration before parent Pushed). Leave path undefined; the + // post-effect fallback applies (fillWithoutEncode). + } + + overrideActionParams({ + ...actionParams, + ...(path !== undefined + ? { + stepContext: { + ...(actionParams.stepContext as Record), + path, + }, + } + : {}), + stepParams: coerceParamsToString(actionParams.stepParams), + }); + }, + onBeforeStepReplace({ + actionParams, + actions: { getStack, overrideActionParams }, + }) { + // FEP-1061 (Option B, B5): mirror onBeforeStepPush. + const ctx = actionParams.stepContext as + | { path?: string } + | undefined; + const hasExistingPath = !!ctx && "path" in ctx; + + let path: string | undefined; + if (hasExistingPath) { + path = ctx?.path; + } else { + const stack = getStack(); + const activeActivity = stack.activities.find((a) => a.isActive); + if (activeActivity) { + const match = activityRoutes.find( + (r) => r.activityName === activeActivity.name, + ); + if (match) { + const template = makeTemplate(match, options.urlPatternOptions); + path = template.fill(actionParams.stepParams); + } + } + } + + overrideActionParams({ + ...actionParams, + ...(path !== undefined + ? { + stepContext: { + ...(actionParams.stepContext as Record), + path, + }, + } + : {}), + stepParams: coerceParamsToString(actionParams.stepParams), + }); + }, onBeforeStepPop({ actions: { getStack } }) { const { activities } = getStack(); const currentActivity = activities.find( diff --git a/extensions/plugin-history-sync/src/makeTemplate.spec.ts b/extensions/plugin-history-sync/src/makeTemplate.spec.ts index 376e6922d..9b6b566fa 100644 --- a/extensions/plugin-history-sync/src/makeTemplate.spec.ts +++ b/extensions/plugin-history-sync/src/makeTemplate.spec.ts @@ -93,3 +93,173 @@ test("makeTemplate - fill with encode function using JSON.stringify for object p "/search/?filter=%7B%22category%22%3A%22tech%22%2C%22tags%22%3A%5B%22javascript%22%2C%22react%22%5D%7D", ); }); + +test("makeTemplate - fill still calls encode with typed params (not pre-stringified)", () => { + const encode = jest.fn((params: Record) => ({ + visible: params.visible ? "y" : "n", + })); + const template = makeTemplate({ + path: "/toggle", + encode, + }); + + const url = template.fill({ visible: true }); + + expect(encode).toHaveBeenCalledTimes(1); + // The boolean is preserved into encode — not pre-stringified to "true". + expect(encode).toHaveBeenCalledWith({ visible: true }); + expect(url).toEqual("/toggle/?visible=y"); +}); + +test("makeTemplate - fillWithoutEncode skips encode entirely", () => { + const encode = jest.fn((params: Record) => ({ + visible: params.visible ? "y" : "n", + })); + const template = makeTemplate({ + path: "/toggle", + encode, + }); + + const url = template.fillWithoutEncode({ visible: "true" }); + + expect(encode).not.toHaveBeenCalled(); + expect(url).toEqual("/toggle/?visible=true"); +}); + +test("makeTemplate - fillWithoutEncode interpolates path params", () => { + const template = makeTemplate({ path: "/articles/:articleId" }); + + expect( + template.fillWithoutEncode({ + articleId: "1234", + title: "hello", + }), + ).toEqual("/articles/1234/?title=hello"); +}); + +test("makeTemplate - fillWithoutEncode drops undefined values", () => { + const template = makeTemplate({ path: "/articles" }); + + expect( + template.fillWithoutEncode({ + articleId: "1234", + test: undefined, + }), + ).toEqual("/articles/?articleId=1234"); +}); + +test("makeTemplate - fill and fillWithoutEncode diverge under NON-IDENTITY encode (fillWithoutEncode does NOT call encode) (T-M1)", () => { + // Replaces the previous vacuous identity-encode parity test. Identity + // encode makes the two paths trivially equal regardless of whether + // `fillWithoutEncode` skips encode or not — so the test couldn't catch + // a regression that made `fillWithoutEncode` accidentally call encode. + // Non-identity encode (boolean → "y"/"n") proves the contract: encode + // mutates the URL when called, so its absence is observable. + const encode = jest.fn((params: Record) => ({ + articleId: String(params.articleId), + visible: params.visible ? "y" : "n", + })); + const template = makeTemplate({ + path: "/articles/:articleId", + encode, + }); + + // fillWithoutEncode receives already-stringified params and MUST skip encode. + const urlWithoutEncode = template.fillWithoutEncode({ + articleId: "1234", + visible: "true", + }); + expect(encode).not.toHaveBeenCalled(); + expect(urlWithoutEncode).toEqual("/articles/1234/?visible=true"); + + // fill on the typed equivalent calls encode and produces a DIFFERENT URL. + encode.mockClear(); + const urlWithEncode = template.fill({ articleId: "1234", visible: true }); + expect(encode).toHaveBeenCalledTimes(1); + expect(urlWithEncode).toEqual("/articles/1234/?visible=y"); + + // The two URLs MUST diverge — proving the test is not vacuous. + expect(urlWithoutEncode).not.toEqual(urlWithEncode); +}); + +test("makeTemplate - fill(typed) === fillWithoutEncode(coerced(encode(typed))) — non-identity drift theorem (T-M1)", () => { + // Replaces the second vacuous identity-encode parity test. With a + // non-identity encode, `fill(typed)` must equal building a URL from the + // already-encoded-and-then-stringified store params via + // `fillWithoutEncode`. This is the round-trip property FEP-1061 relies + // on for `onPushed` to reproduce the same URL using the coerced store + // params. + const encode = jest.fn((params: Record) => ({ + articleId: String(params.articleId), + visible: params.visible ? "y" : "n", + })); + const template = makeTemplate({ + path: "/articles/:articleId", + encode, + }); + + const typed = { articleId: "1234", visible: true }; + const fillUrl = template.fill(typed); + + // Mirror what FEP-1061 does at runtime: encode runs on the typed params, + // then the encoded values are coerced (here they are already strings, + // but we exercise the same shape `fillWithoutEncode` would receive from + // the store). + const encoded = encode(typed); + encode.mockClear(); + const fillWithoutEncodeUrl = template.fillWithoutEncode(encoded); + + // fillWithoutEncode must NOT have called encode again. + expect(encode).not.toHaveBeenCalled(); + // Both paths must yield the same URL. + expect(fillUrl).toEqual(fillWithoutEncodeUrl); +}); + +test("makeTemplate - fillWithoutEncode with empty-string value drops the key from the URL query (falsy guard in _buildUrl)", () => { + // `_buildUrl` has a `value ? { [key]: value } : null` reducer which treats + // "" as falsy and therefore omits the key from the search params entirely. + // This test documents that empty strings are NOT written to the URL query, + // even though they are valid `string` values in the store. + const template = makeTemplate({ path: "/articles" }); + + expect( + template.fillWithoutEncode({ + articleId: "1234", + empty: "", + }), + ).toEqual("/articles/?articleId=1234"); +}); + +test("makeTemplate - fill propagates synchronous errors from user-supplied encode (does not catch)", () => { + const template = makeTemplate({ + path: "/toggle", + encode: () => { + throw new Error("encode boom"); + }, + }); + + // `fill` does not wrap `encode` in try/catch — user errors propagate. + expect(() => template.fill({ visible: true })).toThrow("encode boom"); +}); + +test("makeTemplate - parse with custom decode receives raw URL strings unchanged (decode input is pre-coercion)", () => { + const decode = jest.fn((params: Record) => ({ + articleId: params.articleId, + enabled: params.enabled === "y", + })); + const template = makeTemplate({ + path: "/articles/:articleId", + decode, + }); + + template.parse("/articles/1234/?enabled=y&empty="); + + // `decode` must be called with the raw URL-derived strings — no prior + // type coercion. The empty-string value from the query is preserved as "". + expect(decode).toHaveBeenCalledTimes(1); + expect(decode).toHaveBeenCalledWith({ + articleId: "1234", + enabled: "y", + empty: "", + }); +}); diff --git a/extensions/plugin-history-sync/src/makeTemplate.ts b/extensions/plugin-history-sync/src/makeTemplate.ts index acd8618dc..aa4e34c51 100644 --- a/extensions/plugin-history-sync/src/makeTemplate.ts +++ b/extensions/plugin-history-sync/src/makeTemplate.ts @@ -57,38 +57,70 @@ export function makeTemplate( ? Number.POSITIVE_INFINITY : (pattern as any).names.length; + /** + * Build a URL from already-encoded (string-shaped) params. + * + * Shared internal helper used by both {@link fill} (encode-aware path) and + * {@link fillWithoutEncode} (encode-free path). Centralizing URL-building + * here prevents the two public methods from drifting apart (see FEP-1061 + * pre-mortem: Scenario 3). + */ + const _buildUrl = (encodedParams: { [key: string]: string | undefined }) => { + const pathname = pattern.stringify(encodedParams); + const pathParams = pattern.match(pathname); + + const searchParamsMap = { ...encodedParams }; + + Object.keys(pathParams).forEach((key) => { + delete searchParamsMap[key]; + }); + + const searchParams = new URLSearchParams( + Object.entries(searchParamsMap).reduce( + (acc, [key, value]) => ({ + ...acc, + ...(value + ? { + [key]: value, + } + : null), + }), + {} as Record, + ), + ); + + return ( + appendTrailingSlashInPathname(pathname) + + prependQuestionMarkInSearchParams(searchParams) + ); + }; + return { + /** + * Build a URL from typed params, running `encode` (if provided) first. + * + * This is the original fill behavior: `encode` is always called with the + * component-facing typed params `U`. Callers must pass the original typed + * params — NEVER already-stringified store values (use + * {@link fillWithoutEncode} for those). + */ fill(params: { [key: string]: any }) { const encodedParams: { [key: string]: string | undefined } = encode ? encode(params as Parameters[0]) : params; - const pathname = pattern.stringify(encodedParams); - const pathParams = pattern.match(pathname); - - const searchParamsMap = { ...encodedParams }; - - Object.keys(pathParams).forEach((key) => { - delete searchParamsMap[key]; - }); - - const searchParams = new URLSearchParams( - Object.entries(searchParamsMap).reduce( - (acc, [key, value]) => ({ - ...acc, - ...(value - ? { - [key]: value, - } - : null), - }), - {} as Record, - ), - ); - - return ( - appendTrailingSlashInPathname(pathname) + - prependQuestionMarkInSearchParams(searchParams) - ); + return _buildUrl(encodedParams); + }, + /** + * Build a URL from pre-stringified params, skipping `encode`. + * + * Use this when the caller has already stringified the params (e.g. when + * reading `activity.params` from the core store, which FEP-1061 now + * guarantees is `{ [key: string]: string | undefined }`). Calling + * {@link fill} instead would re-run `encode` on already-stringified + * values and violate the `encode` contract. + */ + fillWithoutEncode(params: Record) { + return _buildUrl(params); }, parse( path: string,