Skip to content

Commit fb2c71d

Browse files
yosriadyclaude
andauthored
Emit screen views as a well-formed app://<bundle id>/<screen> URL (#71)
* Emit screen views as a well-formed app://<bundle id>/<screen> URL page_url was app://<screen>, which is not a valid URL: the screen name lands in the authority slot and there is no path at all. Every URL parser therefore returns an empty path, which is why the ingestion pipeline had to reconstruct both origin and page_path by hand instead of using the URL functions it already applies to web events. Now app://com.acme.wallet/Home — bundle id as the authority, screen as the path, mirroring https://host/path. domainWithoutWWW() yields the bundle id and path() yields the screen, with no mobile-specific handling. The bundle id is also the right authority for the same reason a hostname is: stable and globally unique. A display name is neither — renaming an app would split its history, and two apps sharing a name would merge. Falls back to an empty authority (app:///Home) when no bundle id is available, which still parses to a correct path; the pipeline resolves origin from context there. Leading slashes in the screen name are stripped so a router-style name does not produce a doubled separator. getDeviceInfo() is now memoised. Screen events need the bundle id and generateContext needs the same values for every event, so without it each screen view would cost two native bridge round-trips; with it, zero after the first event. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Use the configured bundle id for the screen URL, not just the detected one generateContext lets options.app.bundleId override whatever the native modules report, but the screen URL read getDeviceInfo() directly — so an explicitly configured bundle id was ignored and every screen URL silently fell back to the authority-less form. Caught by running the example app: it configures app.bundleId = "com.formo.analytics.demo" and got "app:///Wallet". On React Native Web that is the only way to get a bundle id at all, since neither react-native-device-info nor expo-application resolves one there. Extracted resolveAppBundleId() so the URL and context.app_bundle_id cannot disagree, and added tests for both precedence orders. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 1d205b0 commit fb2c71d

3 files changed

Lines changed: 183 additions & 35 deletions

File tree

src/__tests__/lifecycleWire.integration.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,10 +240,12 @@ describe("lifecycle events on the wire", () => {
240240
await analytics.screen("Wallet");
241241
await analytics.flush();
242242

243+
// Well-formed: bundle id in the authority, screen in the path, so the
244+
// Tinybird pipe parses both with plain URL functions.
243245
expect(sent[0]).toMatchObject({
244246
type: "page",
245247
channel: "mobile",
246-
context: expect.objectContaining({ page_url: "app://Wallet" }),
248+
context: expect.objectContaining({ page_url: "app://com.test.app/Wallet" }),
247249
});
248250
});
249251

src/__tests__/screenEvent.test.ts

Lines changed: 101 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,77 @@
1-
import { EventFactory } from "../lib/event/EventFactory";
1+
import { EventFactory, buildScreenUrl } from "../lib/event/EventFactory";
22
import { initStorageManager } from "../lib/storage";
33

44
/**
5-
* Mobile screen views are emitted as type="page" with page_url `app://<name>`.
6-
* The ingestion pipeline (P-2070) owns the interpretation: it derives `origin`
7-
* from the app identifier in context (app_name / app_bundle_id) and `page_path`
8-
* by stripping the app:// scheme. So the SDK emits the screen name as-is and
9-
* must NOT encode an app host in the URL — doing so would leak the host into the
10-
* backend-derived page_path.
5+
* Mobile screen views are emitted as type="page" with
6+
* page_url = `app://<bundle id>/<screen>`.
7+
*
8+
* The shape is the point: it is a well-formed URL, so the bundle id sits in the
9+
* authority slot and the screen in the path — exactly like `https://<host>/<path>`
10+
* on web. A standard URL parser therefore yields origin and page_path directly,
11+
* with no mobile-specific reconstruction downstream.
12+
*
13+
* The previous form, `app://<screen>`, was malformed: the screen name occupied
14+
* the authority and there was no path at all, so parsers returned an empty path
15+
* and the pipeline had to rebuild both fields by hand.
16+
*
17+
* jest.setup mocks react-native-device-info with getBundleId() -> "com.test.app".
1118
*/
19+
describe("buildScreenUrl", () => {
20+
it("puts the bundle id in the authority and the screen in the path", () => {
21+
expect(buildScreenUrl("com.acme.wallet", "Home")).toBe(
22+
"app://com.acme.wallet/Home",
23+
);
24+
});
25+
26+
it("does not double the separator for router-style names", () => {
27+
expect(buildScreenUrl("com.acme.wallet", "/tabs/leaderboard")).toBe(
28+
"app://com.acme.wallet/tabs/leaderboard",
29+
);
30+
expect(buildScreenUrl("com.acme.wallet", "///deep")).toBe(
31+
"app://com.acme.wallet/deep",
32+
);
33+
});
34+
35+
it("preserves nested screen paths", () => {
36+
expect(buildScreenUrl("com.acme.wallet", "tabs/leaderboard")).toBe(
37+
"app://com.acme.wallet/tabs/leaderboard",
38+
);
39+
});
40+
41+
it("omits the authority when there is no bundle id, keeping the path parseable", () => {
42+
// app:///Home still yields path "/Home"; the pipeline resolves origin from
43+
// context in this case rather than from the URL.
44+
expect(buildScreenUrl("", "Home")).toBe("app:///Home");
45+
});
46+
47+
it("handles an empty screen name as the app root", () => {
48+
expect(buildScreenUrl("com.acme.wallet", "")).toBe("app://com.acme.wallet/");
49+
});
50+
});
51+
1252
describe("generateScreenEvent page_url", () => {
1353
beforeEach(() => initStorageManager("screen-test-key"));
1454

15-
it("emits the screen name as app://<name>", async () => {
55+
it("emits app://<bundle id>/<screen>", async () => {
1656
const factory = new EventFactory();
1757
const evt = await factory.generateScreenEvent("Home");
18-
expect(evt.context?.page_url).toBe("app://Home");
58+
expect(evt.context?.page_url).toBe("app://com.test.app/Home");
1959
expect(evt.context?.page_title).toBe("Home");
2060
});
2161

22-
it("passes router-style paths through unchanged", async () => {
62+
it("normalises router-style paths into the URL path", async () => {
2363
const factory = new EventFactory();
2464
const evt = await factory.generateScreenEvent("/tabs/leaderboard");
25-
expect(evt.context?.page_url).toBe("app:///tabs/leaderboard");
65+
expect(evt.context?.page_url).toBe("app://com.test.app/tabs/leaderboard");
2666
});
2767

28-
it("does NOT encode the app bundle id into the URL (backend owns origin)", async () => {
29-
const factory = new EventFactory({ app: { bundleId: "com.formo.test" } });
68+
it("keeps the screen name out of the authority", async () => {
69+
// The regression the well-formed shape exists to prevent: with the screen
70+
// in the authority slot, URL parsers report an empty path.
71+
const factory = new EventFactory();
3072
const evt = await factory.generateScreenEvent("Wallet");
31-
expect(evt.context?.page_url).toBe("app://Wallet");
32-
expect(String(evt.context?.page_url)).not.toContain("com.formo.test");
73+
expect(evt.context?.page_url).not.toBe("app://Wallet");
74+
expect(String(evt.context?.page_url)).toContain("/Wallet");
3375
});
3476

3577
it("keeps user-supplied context (spread last)", async () => {
@@ -39,4 +81,48 @@ describe("generateScreenEvent page_url", () => {
3981
});
4082
expect(evt.context?.page_url).toBe("app://Custom");
4183
});
84+
85+
it("resolves device info once across repeated screen events", async () => {
86+
// getDeviceInfo() is memoised because it crosses the native bridge and its
87+
// values are static. Screen events now depend on it for the bundle id, and
88+
// generateContext needs it for every event too — without the memo each
89+
// screen view would cost two round-trips instead of zero.
90+
const factory = new EventFactory();
91+
const resolve = jest.spyOn(
92+
factory as unknown as { resolveDeviceInfo: () => Promise<unknown> },
93+
"resolveDeviceInfo",
94+
);
95+
96+
await factory.generateScreenEvent("Home");
97+
await factory.generateScreenEvent("Wallet");
98+
await factory.generateScreenEvent("Settings");
99+
100+
expect(resolve).toHaveBeenCalledTimes(1);
101+
});
102+
});
103+
104+
describe("generateScreenEvent bundle id resolution", () => {
105+
beforeEach(() => initStorageManager("screen-bundle-key"));
106+
107+
it("prefers an explicitly configured options.app.bundleId", async () => {
108+
// generateContext lets options.app.bundleId override the native modules, so
109+
// the URL has to use the same precedence. Reading device info alone would
110+
// ignore the configuration — and on React Native Web nothing else resolves a
111+
// bundle id, so every screen URL would lose its authority.
112+
const factory = new EventFactory({ app: { bundleId: "com.configured.app" } });
113+
const evt = await factory.generateScreenEvent("Wallet");
114+
115+
expect(evt.context?.page_url).toBe("app://com.configured.app/Wallet");
116+
// The URL authority and the context field must not disagree.
117+
expect(evt.context?.app_bundle_id).toBe("com.configured.app");
118+
});
119+
120+
it("falls back to the detected bundle id when none is configured", async () => {
121+
const factory = new EventFactory();
122+
const evt = await factory.generateScreenEvent("Wallet");
123+
124+
expect(evt.context?.page_url).toBe(
125+
`app://${evt.context?.app_bundle_id}/Wallet`,
126+
);
127+
});
42128
});

src/lib/event/EventFactory.ts

Lines changed: 79 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,38 @@ export function getSessionId(): string {
128128
* device breakdowns downstream. Unknown means "mobile", the safe default for a
129129
* React Native app.
130130
*/
131+
interface DeviceInfoResult {
132+
os_name: string;
133+
os_version: string;
134+
device_model: string;
135+
device_manufacturer: string;
136+
device_name: string;
137+
device_type: string;
138+
user_agent: string;
139+
app_name: string;
140+
app_version: string;
141+
app_build: string;
142+
app_bundle_id: string;
143+
}
144+
145+
/**
146+
* Compose a screen view's `page_url` as `app://<bundle id>/<screen>`.
147+
*
148+
* Well-formed on purpose: the bundle id occupies the authority slot and the
149+
* screen the path, so a standard URL parser yields both without any
150+
* mobile-specific handling — the same shape as `https://<host>/<path>`.
151+
*
152+
* @param bundleId Application bundle id, e.g. "com.acme.wallet". When empty the
153+
* authority is omitted (`app:///<screen>`), which still parses to a correct
154+
* path; the pipeline resolves origin from context in that case.
155+
* @param name Screen name. Leading slashes are stripped so a router-style name
156+
* ("/tabs/leaderboard") does not produce a doubled separator.
157+
*/
158+
export function buildScreenUrl(bundleId: string, name: string): string {
159+
const screen = (name ?? "").replace(/^\/+/, "");
160+
return `app://${bundleId ?? ""}/${screen}`;
161+
}
162+
131163
export function resolveExpoDeviceType(
132164
deviceType: number | null | undefined,
133165
tabletEnumValue: number | null | undefined,
@@ -165,6 +197,8 @@ export function synthesizeUserAgent(info: {
165197
*/
166198
class EventFactory implements IEventFactory {
167199
private options?: Options;
200+
/** Memoised device/app identity — see getDeviceInfo(). */
201+
private deviceInfoPromise?: Promise<DeviceInfoResult>;
168202

169203
constructor(options?: Options) {
170204
this.options = options;
@@ -288,19 +322,32 @@ class EventFactory implements IEventFactory {
288322
* Get device information
289323
* Supports both react-native-device-info (bare RN) and expo-device/expo-application (Expo Go)
290324
*/
291-
private async getDeviceInfo(): Promise<{
292-
os_name: string;
293-
os_version: string;
294-
device_model: string;
295-
device_manufacturer: string;
296-
device_name: string;
297-
device_type: string;
298-
user_agent: string;
299-
app_name: string;
300-
app_version: string;
301-
app_build: string;
302-
app_bundle_id: string;
303-
}> {
325+
/**
326+
* The app bundle id as it will appear in context.
327+
*
328+
* Must mirror generateContext's precedence: an explicitly configured
329+
* `options.app.bundleId` overrides whatever the native modules report. Reading
330+
* getDeviceInfo() alone would ignore that configuration and silently fall back
331+
* to the authority-less URL form — and on React Native Web, where neither
332+
* react-native-device-info nor expo-application resolves a bundle id, that is
333+
* the ONLY value available.
334+
*/
335+
private async resolveAppBundleId(): Promise<string> {
336+
return (
337+
this.options?.app?.bundleId || (await this.getDeviceInfo()).app_bundle_id || ""
338+
);
339+
}
340+
341+
private async getDeviceInfo(): Promise<DeviceInfoResult> {
342+
// Device and app identity do not change for the lifetime of the process,
343+
// and resolving them crosses the native bridge. Memoise the promise so
344+
// every event after the first is free, and so callers that need only the
345+
// app identifier (screen events) can ask without paying twice.
346+
this.deviceInfoPromise ??= this.resolveDeviceInfo();
347+
return this.deviceInfoPromise;
348+
}
349+
350+
private async resolveDeviceInfo(): Promise<DeviceInfoResult> {
304351
// Try react-native-device-info first (bare RN and Expo dev builds)
305352
if (DeviceInfo) {
306353
try {
@@ -509,15 +556,28 @@ class EventFactory implements IEventFactory {
509556
const props = { ...(properties ?? {}), name, ...(category && { category }) };
510557

511558
// Map screen name to page-equivalent context fields so mobile screens flow
512-
// through the same analytics as web page views. The screen name is emitted
513-
// as-is in the app:// URL; the ingestion pipeline derives `origin` (from the
514-
// app identifier in context — app_name / app_bundle_id) and `page_path` (by
515-
// stripping the app:// scheme), so the SDK deliberately does NOT encode a
516-
// host here (see backend mobile page-event handling, P-2070).
559+
// through the same analytics as web page views.
560+
//
561+
// The URL is app://<bundle id>/<screen>, which is a WELL-FORMED URL: the
562+
// bundle id is the authority and the screen is the path, exactly mirroring
563+
// https://<host>/<path> on web. That is what lets the ingestion pipeline
564+
// parse it with the same URL functions it uses for web — the authority
565+
// becomes `origin` and the path becomes `page_path`, with no mobile
566+
// special-casing.
567+
//
568+
// The earlier form was app://<screen>, which is malformed: the screen name
569+
// lands in the authority slot and there is no path at all, so a URL parser
570+
// yields an empty path and the pipeline had to reconstruct both fields by
571+
// hand. The bundle id is also the right choice of authority because, like a
572+
// hostname, it is stable and globally unique — a display name is neither.
573+
//
574+
// Falls back to an empty authority (app:///<screen>) when the bundle id is
575+
// unavailable, which keeps the path parseable and lets the pipeline resolve
576+
// origin from context instead.
517577
// User-supplied context values take precedence (spread last).
518578
const screenContext: IFormoEventContext = {
519579
page_title: name,
520-
page_url: `app://${name}`,
580+
page_url: buildScreenUrl(await this.resolveAppBundleId(), name),
521581
...(context ?? {}),
522582
};
523583

0 commit comments

Comments
 (0)