From 1a60f853ff467844b547832a7ab7a6228c6326e8 Mon Sep 17 00:00:00 2001 From: Javi Aguilar <122741+itsjavi@users.noreply.github.com> Date: Fri, 28 Aug 2026 12:32:46 +0000 Subject: [PATCH 1/3] feat: setEmbeddedData() + clearEmbeddedData() for app surveys [ENG-2472] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A host app can attach context to future responses without tying it to a trigger. `track()` takes a name and nothing else, so today the only way to get context onto a response is to declare it in the survey and have the respondent type it. setEmbeddedData({ screen: "checkout", plan: "pro" }); setEmbeddedData({ screen: null }); // remove one key clearEmbeddedData("plan"); // same, explicitly clearEmbeddedData(); // everything — logout, context switch Merge, never replace, so refreshing a volatile field cannot wipe a stable one. `null` removes a key; `undefined` is a no-op, not an eviction — a host that builds the object from its own state passes every field unconditionally, and a key absent on the current screen must not cost the bag its value. Only a literal zero-argument `clearEmbeddedData()` clears everything. In-memory and never persisted: persisting would blur the Embedded Data ↔ contact-attribute boundary and create a PII-at-rest surface. Cleared on an identity switch and on logout so one user's context cannot ride onto the next user's responses on a shared device; kept on first identification, because a host legitimately pushes context before it knows who the user is. Snapshotted when the survey is displayed and frozen for its lifetime — the same delay-aware moment js-core snapshots at. Held in state rather than read during render: the html string is an input to the WebView, so reading a mutable bag while rendering would let a later write rewrite `source` and reload the WebView mid-survey, losing the respondent's answers. The bag rides the props payload that already exists, under `hiddenFieldsRecord` — no new bridge message, and deliberately so: a `setEmbeddedData` after display must not reach the survey on screen. It is passed raw and unfiltered, because the ingest contract lives in the renderer (allow-list, coercion, `locked`, size caps) and the server re-runs all of it. Filtering here would ship a second copy of those rules for four mobile SDKs to drift from. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018L6PrECN38Jbe1FEdFccTz --- .../src/components/survey-web-view.tsx | 30 +++- .../tests/survey-web-view-harness.test.ts | 20 +++ packages/react-native/src/index.ts | 32 ++++ .../src/lib/survey/embedded-data.ts | 115 ++++++++++++++ .../lib/survey/tests/embedded-data.test.ts | 147 ++++++++++++++++++ .../src/lib/user/tests/user.test.ts | 67 ++++++++ packages/react-native/src/lib/user/user.ts | 10 ++ 7 files changed, 417 insertions(+), 4 deletions(-) create mode 100644 packages/react-native/src/lib/survey/embedded-data.ts create mode 100644 packages/react-native/src/lib/survey/tests/embedded-data.test.ts diff --git a/packages/react-native/src/components/survey-web-view.tsx b/packages/react-native/src/components/survey-web-view.tsx index 47efcc9..0e7c2ac 100644 --- a/packages/react-native/src/components/survey-web-view.tsx +++ b/packages/react-native/src/components/survey-web-view.tsx @@ -11,9 +11,11 @@ import { getSurveyScriptUrl } from "@/components/utils/survey-script-url"; import { RNConfig } from "@/lib/common/config"; import { Logger } from "@/lib/common/logger"; import { filterSurveys, getLanguageCode, getStyling } from "@/lib/common/utils"; +import { EmbeddedDataStore } from "@/lib/survey/embedded-data"; import { SurveyStore } from "@/lib/survey/store"; import { refreshSegmentsAfterInteraction } from "@/lib/user/interaction-refresh"; import { type TUserState, ZJsRNWebViewOnMessageData } from "@/types/config"; +import type { TResponseData } from "@/types/response"; import type { SurveyContainerProps, TSurvey } from "@/types/survey"; const logger = Logger.getInstance(); @@ -31,6 +33,18 @@ export function SurveyWebView(props: SurveyWebViewProps): JSX.Element | null { const [showSurvey, setShowSurvey] = useState(false); const [appConfig, setAppConfig] = useState(null); const [languageCode, setLanguageCode] = useState("default"); + /** + * The Embedded Data bag, snapshotted at the moment the survey is shown and frozen for the rest of + * its life (ENG-1844). Held in state rather than read inline in the render pass on purpose: the + * html string below is an *input* to the WebView, so reading a mutable bag while rendering would + * let a later `setEmbeddedData` rewrite `source` and reload the WebView mid-survey — losing the + * respondent's answers. A later write therefore reaches the next response, never this one. + * + * Set in the same update as `setShowSurvey(true)`, which is the delay-aware display moment + * js-core snapshots at too (it builds its own bag inside the delay timeout). + */ + const [embeddedDataSnapshot, setEmbeddedDataSnapshot] = + useState({}); useEffect(() => { const fetchConfig = async (): Promise => { @@ -74,20 +88,23 @@ export function SurveyWebView(props: SurveyWebViewProps): JSX.Element | null { return; } + const display = (): void => { + setEmbeddedDataSnapshot(EmbeddedDataStore.getInstance().getSnapshot()); + setShowSurvey(true); + }; + if (props.survey.delay) { logger.debug( `Delaying survey "${props.survey.id}" by ${String(props.survey.delay)} seconds`, ); - const timerId = setTimeout(() => { - setShowSurvey(true); - }, props.survey.delay * 1000); + const timerId = setTimeout(display, props.survey.delay * 1000); return () => { clearTimeout(timerId); }; } - setShowSurvey(true); + display(); }, [props.survey.delay, isSurveyRunning, props.survey.id]); if (!appConfig) { @@ -152,6 +169,11 @@ export function SurveyWebView(props: SurveyWebViewProps): JSX.Element | null { clickOutside, overlay, isWebEnvironment: false, + // Passed straight through, unfiltered: the Embedded Data ingest contract lives in + // the renderer (ENG-1845/2472), so all four mobile SDKs inherit the same allow-list, + // coercion and size rules without each shipping a copy. The renderer drops unknown + // and locked keys and logs what it refused; the server re-runs all of it on ingest. + hiddenFieldsRecord: embeddedDataSnapshot, }), }} style={styles.webView} diff --git a/packages/react-native/src/components/tests/survey-web-view-harness.test.ts b/packages/react-native/src/components/tests/survey-web-view-harness.test.ts index 844751c..844a062 100644 --- a/packages/react-native/src/components/tests/survey-web-view-harness.test.ts +++ b/packages/react-native/src/components/tests/survey-web-view-harness.test.ts @@ -44,6 +44,26 @@ describe("WebView harness", () => { expect(propsBlock).toContain("onClose,"); }); + /** + * The Embedded Data bag (ENG-1844/2472) rides the props blob that already exists — no new bridge + * message. The blob is JSON, so this pins that the key survives serialization under the name the + * renderer reads, with the SDK doing no filtering of its own: the renderer owns the allow-list. + */ + test("carries hiddenFieldsRecord into the payload, raw and unfiltered", () => { + const html = renderHtml({ + appUrl: "https://app.formbricks.com", + workspaceId: "ws-1", + hiddenFieldsRecord: { + plan: "pro", + notDeclaredBySurvey: "kept — the renderer decides, not the SDK", + }, + }); + + expect(html).toContain('"hiddenFieldsRecord":{'); + expect(html).toContain('"plan":"pro"'); + expect(html).toContain('"notDeclaredBySurvey"'); + }); + test("still escapes < in the payload so survey content cannot break out of the script", () => { const html = renderHtml({ appUrl: "https://app.formbricks.com", diff --git a/packages/react-native/src/index.ts b/packages/react-native/src/index.ts index 37ec88f..290f5e6 100644 --- a/packages/react-native/src/index.ts +++ b/packages/react-native/src/index.ts @@ -1,6 +1,10 @@ import { CommandQueue } from "@/lib/common/command-queue"; import { Logger } from "@/lib/common/logger"; import * as Actions from "@/lib/survey/action"; +import { + EmbeddedDataStore, + type TEmbeddedDataInput, +} from "@/lib/survey/embedded-data"; import * as Attributes from "@/lib/user/attribute"; import * as User from "@/lib/user/user"; @@ -43,4 +47,32 @@ export const logout = async (): Promise => { await queue.wait(); }; +/** + * Attach Embedded Data to future responses without tying it to a trigger (ENG-1844). Merges into the + * in-memory bag, last write wins per key; `{ key: null }` removes a key and `undefined` values are + * skipped. Values land only on the survey's declared *ingested* fields — anything else is dropped + * and logged by the renderer, never fatal. + * + * Synchronous and network-free on purpose, unlike the queued methods above: calling it on every + * screen change is free, and routing it through the command queue would silently drop calls made + * before `setup()` completes — a host legitimately pushes context before the SDK is ready. + * + * ```ts + * setEmbeddedData({ screen: "checkout", plan: "pro" }); + * setEmbeddedData({ screen: null }); // remove one key + * ``` + */ +export const setEmbeddedData = (data: TEmbeddedDataInput): void => { + EmbeddedDataStore.getInstance().setEmbeddedData(data); +}; + +/** + * Remove one Embedded Data key, or clear the whole bag when called with no argument — logout, or a + * hard context switch. Synchronous, no network. A key that evaluated to `undefined` is a no-op, not + * a full clear: the arity is forwarded, so only a literal zero-argument call wipes everything. + */ +export const clearEmbeddedData = (...args: [] | [key: string]): void => { + EmbeddedDataStore.getInstance().clearEmbeddedData(...args); +}; + export { Formbricks, Formbricks as default } from "@/components/formbricks"; diff --git a/packages/react-native/src/lib/survey/embedded-data.ts b/packages/react-native/src/lib/survey/embedded-data.ts new file mode 100644 index 0000000..b004f91 --- /dev/null +++ b/packages/react-native/src/lib/survey/embedded-data.ts @@ -0,0 +1,115 @@ +import { Logger } from "@/lib/common/logger"; +import type { TResponseData } from "@/types/response"; + +/** What a host app may hand to `setEmbeddedData`. `null` removes the key; `undefined` is a no-op. */ +export type TEmbeddedDataInput = Record< + string, + string | number | boolean | Date | null | undefined +>; + +/** + * The in-memory Embedded Data bag (ENG-1844/2472): context a host app attaches to future responses + * without tying it to a trigger — `setEmbeddedData({ screen: "checkout" })` once, instead of + * repeating the same values on every possible `track()` call. Mirrors js-core's store so the web and + * mobile SDKs behave identically, key for key. + * + * Lifetime rules, all deliberate: + * + * - **In-memory, process scoped, never persisted.** Not `RNConfig`: that class writes to async + * storage, and persisting this bag would blur the Embedded Data ↔ contact-attribute boundary and + * create a stale-data / PII-at-rest surface. A cold app start begins empty; the host re-pushes. + * - **Snapshot at display, then frozen.** `SurveyWebView` copies the bag into the survey's + * `hiddenFieldsRecord` when the survey is shown; a later `setEmbeddedData` affects the next + * response, never the one on screen. + * - **No filtering here.** The SDK is a dumb pipe: the renderer applies the ingest contract — + * allow-list, coercion, `locked`, size caps — and logs what it refuses, and the server re-runs all + * of it on ingest. Filtering here would ship a second copy of those rules for the four mobile SDKs + * to drift from. + * - **No network.** Every method is a synchronous memory write, so calling `setEmbeddedData` on + * every screen change is free. Values ride the existing response payload. + * + * Backed by a `Map` rather than a plain object so a `__proto__` key is stored as data instead of + * vanishing into the prototype — the same hole the ingest contract closes on the renderer side. + */ +export class EmbeddedDataStore { + private static instance: EmbeddedDataStore | undefined; + private data = new Map(); + + static getInstance(): EmbeddedDataStore { + EmbeddedDataStore.instance ??= new EmbeddedDataStore(); + return EmbeddedDataStore.instance; + } + + /** + * Merge — never replace — so refreshing a volatile field (`screen`) cannot wipe the stable ones + * (`plan`) set at launch. Per key: last write wins; `null` removes; `undefined` does nothing. + * + * The `undefined` no-op is a documented promise, not an accident: a host that builds the object + * from its own state passes every field unconditionally, so a key that is absent on the current + * screen arrives as `undefined` and must not clear the value a previous screen set. + */ + public setEmbeddedData(data: TEmbeddedDataInput): void { + // Guarded rather than thrown: this is a synchronous entry point outside the command queue's + // shield, and a host can legitimately hand over a value that was not there. A broken host build + // is a worse failure than a skipped write. An array is refused too (`typeof [] === "object"`): + // it would spread into junk numeric keys ({0: "a", 1: "b"}). + if (typeof data !== "object" || data === null || Array.isArray(data)) { + Logger.getInstance().error( + `setEmbeddedData: expected an object, got ${data === null ? "null" : typeof data} — nothing was set`, + ); + return; + } + + for (const [key, value] of Object.entries(data)) { + if (value === undefined) continue; + if (value === null) { + this.data.delete(key); + continue; + } + this.data.set(key, value); + } + } + + /** + * Remove one key, or everything when called with no argument (logout / hard context switch). + * + * "No argument" and "an argument that evaluated to `undefined`" are deliberately different: a host + * reading the key from its own state must not wipe the whole bag when that state is empty, so + * only a literal zero-argument call clears everything. + */ + public clearEmbeddedData(...args: [] | [key: string]): void { + if (args.length === 0) { + this.data.clear(); + return; + } + + const [key] = args; + if (typeof key !== "string") { + Logger.getInstance().error( + "clearEmbeddedData: expected a field name — nothing was cleared (call with no argument to clear everything)", + ); + return; + } + + this.data.delete(key); + } + + /** + * A detached copy for the display-time snapshot: mutating the bag after a survey rendered must not + * reach that survey's response. `Object.fromEntries` defines own properties, so a `__proto__` key + * survives the conversion as data. + * + * Dates are serialized here rather than at the JSON boundary: `renderHtml` stringifies the props + * blob, and an ISO 8601 string is exactly what the renderer's ingest contract accepts for a `date` + * field. The cast is the same one js-core makes — the contract accepts boolean and date scalars + * that the narrower legacy `hiddenFields` type cannot spell, and normalizes them before storage. + */ + public getSnapshot(): TResponseData { + return Object.fromEntries( + Array.from(this.data, ([key, value]) => [ + key, + value instanceof Date ? value.toISOString() : value, + ]), + ) as TResponseData; + } +} diff --git a/packages/react-native/src/lib/survey/tests/embedded-data.test.ts b/packages/react-native/src/lib/survey/tests/embedded-data.test.ts new file mode 100644 index 0000000..9408888 --- /dev/null +++ b/packages/react-native/src/lib/survey/tests/embedded-data.test.ts @@ -0,0 +1,147 @@ +import { beforeEach, describe, expect, test, vi } from "vitest"; +import { EmbeddedDataStore } from "@/lib/survey/embedded-data"; + +// The guards log through Logger; mocked so refused inputs don't spray the test output. +vi.mock("@/lib/common/logger", () => ({ + Logger: { getInstance: vi.fn(() => ({ error: vi.fn(), debug: vi.fn() })) }, +})); + +type TSetInput = Parameters[0]; + +describe("EmbeddedDataStore", () => { + let store: EmbeddedDataStore; + + beforeEach(() => { + store = EmbeddedDataStore.getInstance(); + store.clearEmbeddedData(); + }); + + test("merges instead of replacing: setting one key keeps the others", () => { + store.setEmbeddedData({ plan: "pro", screen: "product" }); + store.setEmbeddedData({ screen: "checkout" }); + + expect(store.getSnapshot()).toEqual({ plan: "pro", screen: "checkout" }); + }); + + test("null drops the key", () => { + store.setEmbeddedData({ plan: "pro", screen: "product" }); + store.setEmbeddedData({ screen: null }); + + expect(store.getSnapshot()).toEqual({ plan: "pro" }); + }); + + // One keystroke away from `null` and the opposite behavior: a host that builds the object from + // its own state passes every field unconditionally, so a key absent on the current screen + // arrives as `undefined` and must not clear what a previous screen set. + test("undefined is a no-op — does not set, does not drop", () => { + store.setEmbeddedData({ plan: "pro" }); + store.setEmbeddedData({ plan: undefined, screen: undefined }); + + expect(store.getSnapshot()).toEqual({ plan: "pro" }); + }); + + test("clearEmbeddedData(key) removes one key, clearEmbeddedData() removes everything", () => { + store.setEmbeddedData({ plan: "pro", screen: "product", seats: 4 }); + + store.clearEmbeddedData("screen"); + expect(store.getSnapshot()).toEqual({ plan: "pro", seats: 4 }); + + store.clearEmbeddedData(); + expect(store.getSnapshot()).toEqual({}); + }); + + test("last write wins per key", () => { + store.setEmbeddedData({ plan: "free" }); + store.setEmbeddedData({ plan: "pro" }); + + expect(store.getSnapshot()).toEqual({ plan: "pro" }); + }); + + test("snapshot is a detached copy: later writes do not reach an earlier snapshot", () => { + // The freeze that "a value set after a survey is displayed does not change that response" + // rests on — `SurveyWebView` holds exactly this object for the life of the survey. + store.setEmbeddedData({ plan: "pro" }); + const snapshot = store.getSnapshot(); + + store.setEmbeddedData({ plan: "enterprise", extra: "later" }); + + expect(snapshot).toEqual({ plan: "pro" }); + }); + + test("booleans survive and dates serialize to ISO 8601, which the ingest contract accepts", () => { + store.setEmbeddedData({ + isTrial: true, + signedUpAt: new Date("2026-08-20T10:00:00.000Z"), + }); + + expect(store.getSnapshot()).toEqual({ + isTrial: true, + signedUpAt: "2026-08-20T10:00:00.000Z", + }); + }); + + test("a __proto__ key is stored as data, not swallowed by the prototype", () => { + store.setEmbeddedData({ ["__proto__"]: "value" }); + + const snapshot = store.getSnapshot(); + // Read through the descriptor rather than the dot: it proves the key landed as an own DATA + // property, which `snapshot.__proto__` cannot distinguish from the inherited accessor. + expect(Object.getOwnPropertyDescriptor(snapshot, "__proto__")?.value).toBe( + "value", + ); + expect(Object.keys({})).toEqual([]); + }); + + test("makes no network calls — it is a synchronous memory write", () => { + const fetchMock = vi.fn(); + vi.stubGlobal("fetch", fetchMock); + + store.setEmbeddedData({ plan: "pro", secret: "value" }); + store.clearEmbeddedData("secret"); + store.getSnapshot(); + store.clearEmbeddedData(); + + expect(fetchMock).not.toHaveBeenCalled(); + vi.unstubAllGlobals(); + }); +}); + +describe("input guards (never fatal)", () => { + let store: EmbeddedDataStore; + + beforeEach(() => { + store = EmbeddedDataStore.getInstance(); + store.clearEmbeddedData(); + }); + + test("setEmbeddedData(null) and (undefined) do not throw into host code and set nothing", () => { + store.setEmbeddedData({ plan: "pro" }); + + expect(() => { + store.setEmbeddedData(null as unknown as TSetInput); + store.setEmbeddedData(undefined as unknown as TSetInput); + }).not.toThrow(); + + expect(store.getSnapshot()).toEqual({ plan: "pro" }); + }); + + test("a primitive argument is refused instead of spreading into junk keys", () => { + store.setEmbeddedData("plan" as unknown as TSetInput); + + expect(store.getSnapshot()).toEqual({}); + }); + + test('an array is refused too — typeof [] is "object", but it would spread into numeric junk keys', () => { + store.setEmbeddedData(["a", "b"] as unknown as TSetInput); + + expect(store.getSnapshot()).toEqual({}); + }); + + test("clearEmbeddedData(undefined) is a no-op, NOT a full clear — one keystroke from the no-arg overload", () => { + store.setEmbeddedData({ plan: "pro", screen: "product" }); + + store.clearEmbeddedData(undefined as unknown as string); + + expect(store.getSnapshot()).toEqual({ plan: "pro", screen: "product" }); + }); +}); diff --git a/packages/react-native/src/lib/user/tests/user.test.ts b/packages/react-native/src/lib/user/tests/user.test.ts index 3695c66..e69a813 100644 --- a/packages/react-native/src/lib/user/tests/user.test.ts +++ b/packages/react-native/src/lib/user/tests/user.test.ts @@ -9,6 +9,7 @@ import { import { RNConfig } from "@/lib/common/config"; import { Logger } from "@/lib/common/logger"; import { tearDown } from "@/lib/common/setup"; +import { EmbeddedDataStore } from "@/lib/survey/embedded-data"; import { UpdateQueue } from "@/lib/user/update-queue"; import { logout, setUserId } from "@/lib/user/user"; @@ -209,4 +210,70 @@ describe("user.ts", () => { expect(result.ok).toBe(true); }); }); + + /** + * The ambient Embedded Data bag survives a survey, so it has to be cleared where identity + * changes — otherwise one user's context rides onto the next user's responses on a shared + * device. Deliberately NOT cleared on first identification: a host legitimately pushes context + * before it knows who the user is. + */ + describe("Embedded Data bag on identity change", () => { + const store = (): EmbeddedDataStore => EmbeddedDataStore.getInstance(); + + const mockDeps = (currentUserId: string | null): void => { + getInstanceConfigMock.mockReturnValue({ + get: vi + .fn() + .mockReturnValue({ user: { data: { userId: currentUserId } } }), + } as unknown as Promise); + getInstanceLoggerMock.mockReturnValue({ + debug: vi.fn(), + error: vi.fn(), + } as unknown as Logger); + getInstanceUpdateQueueMock.mockReturnValue({ + updateUserId: vi.fn(), + processUpdates: vi.fn(), + } as unknown as UpdateQueue); + }; + + beforeEach(() => { + store().clearEmbeddedData(); + }); + + test("switching to a different userId clears the bag", async () => { + mockDeps("existing-user"); + store().setEmbeddedData({ plan: "pro" }); + + await setUserId(mockUserId); + + expect(store().getSnapshot()).toEqual({}); + }); + + test("identifying for the first time keeps the bag", async () => { + mockDeps(null); + store().setEmbeddedData({ plan: "pro" }); + + await setUserId(mockUserId); + + expect(store().getSnapshot()).toEqual({ plan: "pro" }); + }); + + test("setting the same userId again keeps the bag", async () => { + mockDeps(mockUserId); + store().setEmbeddedData({ plan: "pro" }); + + await setUserId(mockUserId); + + expect(store().getSnapshot()).toEqual({ plan: "pro" }); + }); + + test("logout clears the bag", async () => { + mockDeps(mockUserId); + store().setEmbeddedData({ plan: "pro" }); + + await logout(); + + expect(store().getSnapshot()).toEqual({}); + }); + }); }); diff --git a/packages/react-native/src/lib/user/user.ts b/packages/react-native/src/lib/user/user.ts index 60c5385..6e52a87 100644 --- a/packages/react-native/src/lib/user/user.ts +++ b/packages/react-native/src/lib/user/user.ts @@ -1,6 +1,7 @@ import { RNConfig } from "@/lib/common/config"; import { Logger } from "@/lib/common/logger"; import { tearDown } from "@/lib/common/setup"; +import { EmbeddedDataStore } from "@/lib/survey/embedded-data"; import { UpdateQueue } from "@/lib/user/update-queue"; import { type ApiErrorResponse, okVoid, type Result } from "@/types/error"; @@ -27,6 +28,12 @@ export const setUserId = async ( "Different userId is being set, cleaning up previous user state", ); await tearDown(); + // An identity switch: the ambient Embedded Data bag may carry the previous user's context + // (hashed ids and the like), which must not ride onto the next user's responses. Deliberately + // not in tearDown() itself — the setup-error teardown is not an identity switch, and app + // context should survive a setup retry. First-time identification (no currentUserId) keeps the + // bag too: the host legitimately pushes context before identifying. + EmbeddedDataStore.getInstance().clearEmbeddedData(); } updateQueue.updateUserId(userId); @@ -39,5 +46,8 @@ export const logout = async (): Promise> => { logger.debug("Logging out and cleaning user state"); await tearDown(); + // Same identity-switch rule as setUserId above: logout must not let the previous user's ambient + // context leak onto whoever uses the app next. + EmbeddedDataStore.getInstance().clearEmbeddedData(); return okVoid(); }; From 4a419f04f12d4f448f170204dfddd28b58dca917 Mon Sep 17 00:00:00 2001 From: Javi Aguilar <122741+itsjavi@users.noreply.github.com> Date: Fri, 28 Aug 2026 15:17:23 +0000 Subject: [PATCH 2/3] fix: refuse an invalid Date, and stop asserting booleans away [ENG-2472] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings, both real. An invalid Date (`new Date("nope")`, `new Date(NaN)`) passed the input type and reached the store. `getSnapshot` calls `toISOString()` on it, which throws a RangeError — and `getSnapshot` runs inside the effect that displays the survey, so one bad value from host code cost the whole survey rather than the one field. It is now refused at the door and logged, the same shape as the non-finite-number guard the other three SDKs need. This guard is deliberately absent from js-core: there the Date object goes straight to the renderer, which coerces it. Serializing to ISO 8601 is this SDK's own step because the props blob crosses a JSON boundary, so the hazard is this SDK's too. `getSnapshot` also returned `TResponseData` through an assertion, and that type cannot spell booleans — which the bag really does hold and the renderer's ingest contract really does accept (`TIngestableScalar` is `string | number | boolean | Date`). The assertion only hid the mismatch from the compiler while the value flowed on regardless. Replaced with `TIngestedFieldsRecord`, which says what actually travels, and the mirrored `hiddenFieldsRecord` prop widened to match the renderer it mirrors. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018L6PrECN38Jbe1FEdFccTz --- .../src/components/survey-web-view.tsx | 4 ++-- .../src/lib/survey/embedded-data.ts | 24 +++++++++++++++---- .../lib/survey/tests/embedded-data.test.ts | 21 ++++++++++++++++ packages/react-native/src/types/response.ts | 14 +++++++++++ packages/react-native/src/types/survey.ts | 8 +++++-- 5 files changed, 62 insertions(+), 9 deletions(-) diff --git a/packages/react-native/src/components/survey-web-view.tsx b/packages/react-native/src/components/survey-web-view.tsx index 0e7c2ac..fa22199 100644 --- a/packages/react-native/src/components/survey-web-view.tsx +++ b/packages/react-native/src/components/survey-web-view.tsx @@ -15,7 +15,7 @@ import { EmbeddedDataStore } from "@/lib/survey/embedded-data"; import { SurveyStore } from "@/lib/survey/store"; import { refreshSegmentsAfterInteraction } from "@/lib/user/interaction-refresh"; import { type TUserState, ZJsRNWebViewOnMessageData } from "@/types/config"; -import type { TResponseData } from "@/types/response"; +import type { TIngestedFieldsRecord } from "@/types/response"; import type { SurveyContainerProps, TSurvey } from "@/types/survey"; const logger = Logger.getInstance(); @@ -44,7 +44,7 @@ export function SurveyWebView(props: SurveyWebViewProps): JSX.Element | null { * js-core snapshots at too (it builds its own bag inside the delay timeout). */ const [embeddedDataSnapshot, setEmbeddedDataSnapshot] = - useState({}); + useState({}); useEffect(() => { const fetchConfig = async (): Promise => { diff --git a/packages/react-native/src/lib/survey/embedded-data.ts b/packages/react-native/src/lib/survey/embedded-data.ts index b004f91..5deccc3 100644 --- a/packages/react-native/src/lib/survey/embedded-data.ts +++ b/packages/react-native/src/lib/survey/embedded-data.ts @@ -1,5 +1,5 @@ import { Logger } from "@/lib/common/logger"; -import type { TResponseData } from "@/types/response"; +import type { TIngestedFieldsRecord } from "@/types/response"; /** What a host app may hand to `setEmbeddedData`. `null` removes the key; `undefined` is a no-op. */ export type TEmbeddedDataInput = Record< @@ -66,6 +66,17 @@ export class EmbeddedDataStore { this.data.delete(key); continue; } + // Refused rather than stored: `toISOString()` throws a RangeError on an invalid Date, and + // `getSnapshot` runs inside the effect that displays the survey — so one `new Date("nope")` + // from host code would cost the survey, not the field. This guard exists here and not in + // js-core because serializing the Date is this SDK's own step: js-core hands the Date object + // straight to the renderer, which coerces it. Never fatal, always logged. + if (value instanceof Date && Number.isNaN(value.getTime())) { + Logger.getInstance().error( + `setEmbeddedData: "${key}" is an invalid Date — the key was skipped`, + ); + continue; + } this.data.set(key, value); } } @@ -101,15 +112,18 @@ export class EmbeddedDataStore { * * Dates are serialized here rather than at the JSON boundary: `renderHtml` stringifies the props * blob, and an ISO 8601 string is exactly what the renderer's ingest contract accepts for a `date` - * field. The cast is the same one js-core makes — the contract accepts boolean and date scalars - * that the narrower legacy `hiddenFields` type cannot spell, and normalizes them before storage. + * field. `setEmbeddedData` refuses an invalid Date, so `toISOString` here cannot throw. + * + * The return type spells booleans rather than asserting them away: the bag really does hold them, + * the renderer's contract really does accept them, and only the legacy `TResponseData` could not + * say so. */ - public getSnapshot(): TResponseData { + public getSnapshot(): TIngestedFieldsRecord { return Object.fromEntries( Array.from(this.data, ([key, value]) => [ key, value instanceof Date ? value.toISOString() : value, ]), - ) as TResponseData; + ); } } diff --git a/packages/react-native/src/lib/survey/tests/embedded-data.test.ts b/packages/react-native/src/lib/survey/tests/embedded-data.test.ts index 9408888..9b998ad 100644 --- a/packages/react-native/src/lib/survey/tests/embedded-data.test.ts +++ b/packages/react-native/src/lib/survey/tests/embedded-data.test.ts @@ -80,6 +80,27 @@ describe("EmbeddedDataStore", () => { }); }); + test("an invalid Date is refused rather than costing the survey", () => { + // THE guard: `toISOString()` throws a RangeError on an invalid Date, and `getSnapshot` runs + // inside the effect that displays the survey — so storing one would mean no survey at all, + // not a missing field. + store.setEmbeddedData({ plan: "pro" }); + + store.setEmbeddedData({ signedUpAt: new Date("not a date") }); + + expect(store.getSnapshot()).toEqual({ plan: "pro" }); + expect(() => store.getSnapshot()).not.toThrow(); + }); + + test("a valid Date alongside an invalid one still lands", () => { + store.setEmbeddedData({ + good: new Date("2026-08-20T10:00:00.000Z"), + bad: new Date(Number.NaN), + }); + + expect(store.getSnapshot()).toEqual({ good: "2026-08-20T10:00:00.000Z" }); + }); + test("a __proto__ key is stored as data, not swallowed by the prototype", () => { store.setEmbeddedData({ ["__proto__"]: "value" }); diff --git a/packages/react-native/src/types/response.ts b/packages/react-native/src/types/response.ts index eb3cc25..1d9f2c4 100644 --- a/packages/react-native/src/types/response.ts +++ b/packages/react-native/src/types/response.ts @@ -14,6 +14,20 @@ export type TResponseHiddenFieldValue = Record< string | number | string[] >; +/** + * What the renderer accepts for `hiddenFieldsRecord`: the legacy shape plus the booleans its + * Embedded Data ingest contract normalizes (`TIngestableScalar` is `string | number | boolean | + * Date`) but that the older types cannot spell. Dates are serialized to ISO 8601 before they get + * here, so they arrive as strings. + * + * Its own type rather than a cast to `TResponseData`: the store really does hold booleans, and an + * assertion there would only hide the mismatch from the compiler while the value flows on regardless. + */ +export type TIngestedFieldsRecord = Record< + string, + string | number | boolean | string[] | Record +>; + export interface TResponseUpdate { finished: boolean; data: TResponseData; diff --git a/packages/react-native/src/types/survey.ts b/packages/react-native/src/types/survey.ts index 3f82dc0..d483dd2 100644 --- a/packages/react-native/src/types/survey.ts +++ b/packages/react-native/src/types/survey.ts @@ -1,4 +1,8 @@ -import type { TResponseData, TResponseUpdate } from "@/types/response"; +import type { + TIngestedFieldsRecord, + TResponseData, + TResponseUpdate, +} from "@/types/response"; import type { TFileUploadParams, TUploadFileConfig } from "@/types/storage"; import type { TOverlay } from "./common"; import type { TWorkspaceStyling } from "./workspace"; @@ -47,7 +51,7 @@ export interface SurveyBaseProps { startAtQuestionId?: string; clickOutside?: boolean; overlay?: TOverlay; - hiddenFieldsRecord?: TResponseData; + hiddenFieldsRecord?: TIngestedFieldsRecord; shouldResetQuestionId?: boolean; fullSizeCards?: boolean; } From 941d20baed5e89a121bdf0959c9b5b29547408af Mon Sep 17 00:00:00 2001 From: Javi Aguilar <122741+itsjavi@users.noreply.github.com> Date: Fri, 28 Aug 2026 15:21:00 +0000 Subject: [PATCH 3/3] chore: mark the store's Map readonly and document the display helper [ENG-2472] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two analyser remarks from the last run. SonarQube typescript:S2933 on `EmbeddedDataStore.data`: the Map reference is never reassigned — only mutated through set/delete/clear — so it is readonly. CodeRabbit's docstring check flagged the one undocumented function in the diff, the `display` helper in SurveyWebView. Its two calls have to happen in one update and in that order, which is not obvious from reading them, so the reason is now written down where it applies rather than a few lines up on the state declaration. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018L6PrECN38Jbe1FEdFccTz --- packages/react-native/src/components/survey-web-view.tsx | 8 ++++++++ packages/react-native/src/lib/survey/embedded-data.ts | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/react-native/src/components/survey-web-view.tsx b/packages/react-native/src/components/survey-web-view.tsx index fa22199..37b30ea 100644 --- a/packages/react-native/src/components/survey-web-view.tsx +++ b/packages/react-native/src/components/survey-web-view.tsx @@ -88,6 +88,14 @@ export function SurveyWebView(props: SurveyWebViewProps): JSX.Element | null { return; } + /** + * Shows the survey, taking the Embedded Data snapshot in the same update. + * + * Both calls belong together and in this order: React batches them into one render, so the + * WebView mounts with the bag already frozen rather than mounting empty and re-rendering with + * it — which would change `source` and reload the survey. Called directly, or from the delay + * timeout below, so the snapshot is always taken at the moment the survey actually appears. + */ const display = (): void => { setEmbeddedDataSnapshot(EmbeddedDataStore.getInstance().getSnapshot()); setShowSurvey(true); diff --git a/packages/react-native/src/lib/survey/embedded-data.ts b/packages/react-native/src/lib/survey/embedded-data.ts index 5deccc3..57c992b 100644 --- a/packages/react-native/src/lib/survey/embedded-data.ts +++ b/packages/react-native/src/lib/survey/embedded-data.ts @@ -33,7 +33,7 @@ export type TEmbeddedDataInput = Record< */ export class EmbeddedDataStore { private static instance: EmbeddedDataStore | undefined; - private data = new Map(); + private readonly data = new Map(); static getInstance(): EmbeddedDataStore { EmbeddedDataStore.instance ??= new EmbeddedDataStore();