diff --git a/.changeset/remove-core-step-context-fields.md b/.changeset/remove-core-step-context-fields.md new file mode 100644 index 000000000..23b623d17 --- /dev/null +++ b/.changeset/remove-core-step-context-fields.md @@ -0,0 +1,6 @@ +--- +"@stackflow/core": patch +--- + +Remove the internal optional `stepContext` event fields and `ActivityStep.context` +storage that were added for plugin-history-sync URL preservation. diff --git a/.changeset/withdraw-history-sync-param-coercion.md b/.changeset/withdraw-history-sync-param-coercion.md new file mode 100644 index 000000000..f59f37502 --- /dev/null +++ b/.changeset/withdraw-history-sync-param-coercion.md @@ -0,0 +1,7 @@ +--- +"@stackflow/plugin-history-sync": patch +--- + +Withdraw the activity and step param string coercion introduced in `1.11.0`. +Internal navigation now preserves non-string param values at runtime again, while +URL arrivals continue to use decoded URL params as before. diff --git a/core/src/Stack.ts b/core/src/Stack.ts index 4c116a394..fe2c0dfc2 100644 --- a/core/src/Stack.ts +++ b/core/src/Stack.ts @@ -1,3 +1,4 @@ +import type { BaseDomainEvent } from "event-types/_base"; import type { DomainEvent, PoppedEvent, @@ -18,7 +19,6 @@ 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 616f0c38a..fac85e2d7 100644 --- a/core/src/activity-utils/makeActivityReducer.ts +++ b/core/src/activity-utils/makeActivityReducer.ts @@ -68,7 +68,6 @@ 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, @@ -89,7 +88,6 @@ 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, @@ -109,7 +107,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 83284c935..d8db38d98 100644 --- a/core/src/aggregate.ts +++ b/core/src/aggregate.ts @@ -75,7 +75,6 @@ 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 2ac5f9699..f962b4766 100644 --- a/core/src/event-types/StepPushedEvent.ts +++ b/core/src/event-types/StepPushedEvent.ts @@ -7,7 +7,6 @@ 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 b2231dcaf..39975e4c0 100644 --- a/core/src/event-types/StepReplacedEvent.ts +++ b/core/src/event-types/StepReplacedEvent.ts @@ -7,7 +7,6 @@ export type StepReplacedEvent = BaseDomainEvent< stepParams: { [key: string]: string | undefined; }; - stepContext?: {}; targetActivityId?: string; hasZIndex?: boolean; } diff --git a/docs/pages/docs/advanced/history-sync.en.mdx b/docs/pages/docs/advanced/history-sync.en.mdx index a7ba65fea..89f700174 100644 --- a/docs/pages/docs/advanced/history-sync.en.mdx +++ b/docs/pages/docs/advanced/history-sync.en.mdx @@ -72,22 +72,6 @@ The `historySyncPlugin` accepts two options: `config` and `fallbackActivity`. **Warning** - When mapping activity parameters to path parameters, ensure that the parameter values are URL-safe. If special characters that are not URL-safe are used, query parameters may appear duplicated. -## Runtime Coercion of Activity Params - -Regardless of how an activity is entered, including `push()`, `replace()`, `pushStep()`, `replaceStep()`, 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("ArticleActivity", { 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, 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, such as `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)`. - - In a server-side rendering environment, the `window.location` value is not available, so the initial activity cannot be determined. To set the initial activity, add the path value to the `req.path` field in the `initialContext` of the Stack as follows: diff --git a/docs/pages/docs/advanced/history-sync.ko.mdx b/docs/pages/docs/advanced/history-sync.ko.mdx index 4c213a2f9..ea48f2089 100644 --- a/docs/pages/docs/advanced/history-sync.ko.mdx +++ b/docs/pages/docs/advanced/history-sync.ko.mdx @@ -75,22 +75,6 @@ const { Stack } = stackflow({ 특수문자를 사용하는 경우 query parameter가 중복해서 나타날 수 있어요. -## 액티비티 파라미터의 런타임 강제 변환 - -액티비티에 어떻게 진입했는지와 관계없이, `push()`, `replace()`, `pushStep()`, `replaceStep()`, 또는 `decode` 훅 유무와 무관한 URL 직접 진입 모두에서 `useActivityParams()`로 받는 파라미터는 런타임에 항상 `string | undefined`예요. - -```tsx -// 이전에는 두 경로가 런타임에 서로 다른 타입을 반환했지만, 이제는 동일해요. -push("ArticleActivity", { visible: true }) // 스토어: { visible: "true" } -// URL 진입: /articles/1?visible=true // 스토어: { visible: "true" } -``` - -라우트의 `encode` 훅은 여전히 원본의 타입이 적용된 파라미터를 받아요. 예를 들어 `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)`. - - 서버사이드 렌더링 환경에서는 `window.location` 값이 없으므로 초기 액티비티를 결정할 수 없어요. 초기 액티비티를 결정하려면 다음과 같이 Stack의 `initialContext`에 `req.path` 필드에 path 값을 넣어주세요. diff --git a/extensions/plugin-history-sync/INTENT.md b/extensions/plugin-history-sync/INTENT.md deleted file mode 100644 index d8b7fc55e..000000000 --- a/extensions/plugin-history-sync/INTENT.md +++ /dev/null @@ -1,71 +0,0 @@ -# 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 deleted file mode 100644 index cd3c7063d..000000000 --- a/extensions/plugin-history-sync/src/coerceParamsToString.spec.ts +++ /dev/null @@ -1,254 +0,0 @@ -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 deleted file mode 100644 index 21cd83553..000000000 --- a/extensions/plugin-history-sync/src/coerceParamsToString.ts +++ /dev/null @@ -1,49 +0,0 @@ -/** - * 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.react.spec.tsx b/extensions/plugin-history-sync/src/historySyncPlugin.react.spec.tsx index 46a3145ac..6ea6b694b 100644 --- a/extensions/plugin-history-sync/src/historySyncPlugin.react.spec.tsx +++ b/extensions/plugin-history-sync/src/historySyncPlugin.react.spec.tsx @@ -1,11 +1,9 @@ /** @jest-environment jsdom */ import { defineConfig } from "@stackflow/config"; import { basicRendererPlugin } from "@stackflow/plugin-renderer-basic"; -import type { ActivityComponentType } from "@stackflow/react"; -import { stackflow, useActivity } from "@stackflow/react"; -import { act, render, waitFor } from "@testing-library/react"; +import { stackflow } from "@stackflow/react"; +import { act } from "@testing-library/react"; import { createMemoryHistory } from "history"; -import { useEffect } from "react"; import { hydrateRoot } from "react-dom/client"; import { renderToString } from "react-dom/server"; import { historySyncPlugin } from "./historySyncPlugin"; @@ -38,36 +36,6 @@ function Article() { return
article
; } -type ActivitySnapshot = ReturnType; - -let recordHomeActivity: ((activity: ActivitySnapshot) => void) | undefined; -let recordArticleActivity: ((activity: ActivitySnapshot) => void) | undefined; - -afterEach(() => { - recordHomeActivity = undefined; - recordArticleActivity = undefined; -}); - -const HomeActivity: ActivityComponentType<"Home"> = () => { - const activity = useActivity(); - - useEffect(() => { - recordHomeActivity?.(activity); - }, [activity]); - - return null; -}; - -const ArticleActivity: ActivityComponentType<"Article"> = () => { - const activity = useActivity(); - - useEffect(() => { - recordArticleActivity?.(activity); - }, [activity]); - - return null; -}; - function makeApp({ defaultHistory = "non-empty", }: { @@ -117,38 +85,6 @@ function makeApp({ }); } -function renderHistorySyncStack({ - config, - initialEntry, -}: { - config: Parameters[0]["config"]; - initialEntry: string; -}) { - const history = createMemoryHistory({ - initialEntries: [initialEntry], - }); - - const app = stackflow({ - config, - components: { - Home: HomeActivity, - Article: ArticleActivity, - }, - plugins: [ - basicRendererPlugin(), - historySyncPlugin({ - config, - history, - fallbackActivity: () => "Home", - }), - ], - }); - - render(); - - return history; -} - function renderServerHTML(app: ReturnType) { const originalWindow = global.window; try { @@ -240,137 +176,3 @@ describe("historySyncPlugin - SSR hydration with defaultHistory", () => { } }); }); - -describe("historySyncPlugin - defaultHistory setup through React rendering", () => { - test("historySyncPlugin - FEP-1061: defaultHistory ancestor entries with typed activityParams + stepParams coerce (T-I-NEW-6)", async () => { - const homeActivities: ActivitySnapshot[] = []; - const articleActivities: ActivitySnapshot[] = []; - recordHomeActivity = (activity) => { - homeActivities.push(activity); - }; - recordArticleActivity = (activity) => { - articleActivities.push(activity); - }; - - const config = defineConfig({ - transitionDuration: TRANSITION_DURATION, - activities: [ - { name: "Home", route: "/home/" }, - { - name: "Article", - route: { - path: "/articles/:articleId", - defaultHistory: () => [ - { - activityName: "Home", - activityParams: { - count: 42 as unknown as string, - }, - additionalSteps: [ - { - stepParams: { - offset: 7 as unknown as string, - }, - }, - ], - }, - ], - }, - }, - ], - }); - - renderHistorySyncStack({ - config, - initialEntry: "/articles/9/", - }); - - await waitFor(() => { - const article = articleActivities.find((activity) => activity.isActive); - - expect(article?.params.articleId).toEqual("9"); - }); - - const home = homeActivities.at(-1); - expect(home?.steps[0]?.params.count).toEqual("42"); - expect(typeof home?.steps[0]?.params.count).toEqual("string"); - expect(home?.params.offset).toEqual("7"); - expect(typeof home?.params.offset).toEqual("string"); - }); - - 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 defaultHistory chain. The ancestor - // URL pushed during setup must use Home's route encode, not the current - // Article path. - const homeActivities: ActivitySnapshot[] = []; - const articleActivities: ActivitySnapshot[] = []; - recordHomeActivity = (activity) => { - homeActivities.push(activity); - }; - recordArticleActivity = (activity) => { - articleActivities.push(activity); - }; - - const homeEncode = (params: unknown) => { - const visible = - typeof params === "object" && params !== null - ? (params as { visible?: unknown }).visible - : undefined; - - return { - visible: visible ? "y" : "n", - }; - }; - - const config = defineConfig({ - transitionDuration: TRANSITION_DURATION, - activities: [ - { - name: "Home", - route: { - path: "/home/", - encode: homeEncode, - }, - }, - { - name: "Article", - route: { - path: "/articles/:articleId", - defaultHistory: () => [ - { - activityName: "Home", - activityParams: { - visible: true as unknown as string, - }, - }, - ], - }, - }, - ], - }); - - const history = renderHistorySyncStack({ - config, - initialEntry: "/articles/9/?visible=true", - }); - - await waitFor(() => { - const article = articleActivities.find((activity) => activity.isActive); - - expect(article?.params.articleId).toEqual("9"); - }); - - await act(async () => { - history.back(); - }); - - await waitFor(() => { - const { pathname, search, hash } = history.location; - - expect(homeActivities.some((activity) => activity.isActive)).toEqual( - true, - ); - expect(`${pathname}${search}${hash}`).toEqual("/home/?visible=y"); - }); - }); -}); diff --git a/extensions/plugin-history-sync/src/historySyncPlugin.spec.ts b/extensions/plugin-history-sync/src/historySyncPlugin.spec.ts index 21704c9ef..66a1f6b03 100644 --- a/extensions/plugin-history-sync/src/historySyncPlugin.spec.ts +++ b/extensions/plugin-history-sync/src/historySyncPlugin.spec.ts @@ -6,7 +6,6 @@ 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"; @@ -113,21 +112,6 @@ 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; @@ -1468,7 +1452,7 @@ describe("historySyncPlugin", () => { articleId: "1", }, activityContext: { - promise: new Promise((resolve, _reject) => resolve()), + promise: new Promise((resolve, reject) => resolve()), }, }); @@ -1478,223 +1462,6 @@ 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(); @@ -1745,2179 +1512,4 @@ 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: 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-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 786f993ab..fab01059f 100644 --- a/extensions/plugin-history-sync/src/historySyncPlugin.tsx +++ b/extensions/plugin-history-sync/src/historySyncPlugin.tsx @@ -19,7 +19,6 @@ 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"; @@ -254,19 +253,10 @@ 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" @@ -274,9 +264,6 @@ export function historySyncPlugin< { ...initialState.step.enteredBy, name: "StepPushed" as const, - stepParams: coerceParamsToString( - initialState.step.enteredBy.stepParams, - ), }, ] : []), @@ -342,88 +329,54 @@ 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: targetActivityRoute.activityName, - activityParams: coerceParamsToString(targetParams), + activityName, + activityParams: { + ...activityParams, + }, activityContext: { - path: targetPath, + path: currentPath, 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([ @@ -512,17 +465,10 @@ 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: activityPath, + pathname: template.fill(activity.params), state: { activity: activity, }, @@ -531,7 +477,7 @@ export function historySyncPlugin< } else { pushState({ history, - pathname: activityPath, + pathname: template.fill(activity.params), state: { activity: activity, }, @@ -541,14 +487,9 @@ 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: stepPath, + pathname: template.fill(step.params), state: { activity: activity, step: step, @@ -673,11 +614,6 @@ 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()) { @@ -689,9 +625,6 @@ 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, }); } }; @@ -710,20 +643,11 @@ 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, + pathname: template.fill(activity.params), state: { activity, }, @@ -743,20 +667,11 @@ 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, + pathname: template.fill(activity.params), state: { activity, step, @@ -776,16 +691,11 @@ 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, + pathname: template.fill(activity.params), state: { activity, }, @@ -804,16 +714,11 @@ 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, + pathname: template.fill(activity.params), state: { activity, step, @@ -823,71 +728,47 @@ export function historySyncPlugin< }); }, onBeforePush({ actionParams, actions: { overrideActionParams } }) { - const needsPath = + if ( !actionParams.activityContext || - "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) { + "path" in actionParams.activityContext === false + ) { const match = activityRoutes.find( (r) => r.activityName === actionParams.activityName, )!; const template = makeTemplate(match, options.urlPatternOptions); - path = template.fill(actionParams.activityParams); - } + const 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), - }); + overrideActionParams({ + ...actionParams, + activityContext: { + ...actionParams.activityContext, + path, + }, + }); + } }, onBeforeReplace({ actionParams, actions: { overrideActionParams, getStack }, }) { - const needsPath = + if ( !actionParams.activityContext || - "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) { + "path" in actionParams.activityContext === false + ) { const match = activityRoutes.find( (r) => r.activityName === actionParams.activityName, )!; const template = makeTemplate(match, options.urlPatternOptions); - path = template.fill(actionParams.activityParams); - } + const path = template.fill(actionParams.activityParams); - overrideActionParams({ - ...actionParams, - ...(needsPath - ? { - activityContext: { - ...actionParams.activityContext, - path, - }, - } - : {}), - activityParams: coerceParamsToString(actionParams.activityParams), - }); + overrideActionParams({ + ...actionParams, + activityContext: { + ...actionParams.activityContext, + path, + }, + }); + } const { activities } = getStack(); const enteredActivities = activities.filter( @@ -918,96 +799,6 @@ 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 9b6b566fa..376e6922d 100644 --- a/extensions/plugin-history-sync/src/makeTemplate.spec.ts +++ b/extensions/plugin-history-sync/src/makeTemplate.spec.ts @@ -93,173 +93,3 @@ 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 aa4e34c51..acd8618dc 100644 --- a/extensions/plugin-history-sync/src/makeTemplate.ts +++ b/extensions/plugin-history-sync/src/makeTemplate.ts @@ -57,70 +57,38 @@ 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; - 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); + 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) + ); }, parse( path: string,