Conversation
Screen events from the React Native SDK now send type="page" instead of type="screen", with screen name mapped to page_title, page_path, and page_url context fields. This allows Tinybird's process_sessions and process_sources materializations (which filter on type='page') to include mobile screen views. The channel="mobile" field distinguishes mobile from web traffic. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
yosriady
changed the title
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| const screenContext: IFormoEventContext = { | ||
| ...(context ?? {}), | ||
| page_title: name, | ||
| page_path: `/${name}`, | ||
| page_url: `app://${name}`, | ||
| }; |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Devin Review found 1 potential issue.
⚠️ 1 issue in files not directly in the diff
⚠️ getActionDescriptor screen branch is now dead code due to type change from "screen" to "page" (src/utils/helpers.ts:113-115)
The PR changes the event type for screen events from "screen" to "page" in EventFactory.generateScreenEvent (src/lib/event/EventFactory.ts:398), but the getActionDescriptor helper at src/utils/helpers.ts:113 still checks for type === "screen". Since screen events now have type: "page", this branch will never match, and EventQueue.enqueue at src/lib/event/EventQueue.ts:168 will log "Event enqueued: page" instead of the more descriptive "Event enqueued: screen:HomeScreen". The screen name is lost from debug logs.
View 3 additional findings in Devin Review.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the EventFactory to map mobile screen view events to page-equivalent context fields, enabling unified analytics processing in Tinybird. The review identified an issue where the new context fields were overwriting existing values provided in the input; a suggestion was provided to prioritize the existing context and improve URI safety.
| const screenContext: IFormoEventContext = { | ||
| ...(context ?? {}), | ||
| page_title: name, | ||
| page_path: `/${name}`, | ||
| page_url: `app://${name}`, | ||
| }; |
There was a problem hiding this comment.
The current implementation of screenContext overwrites any page_title, page_path, or page_url fields that might have been explicitly provided in the context argument. It is better to allow manually provided context to take precedence over these defaults. Additionally, consider that if the screen name contains spaces or special characters, the resulting page_url and page_path might not be valid URIs; you might want to ensure they are properly encoded (e.g., using encodeURIComponent).
| const screenContext: IFormoEventContext = { | |
| ...(context ?? {}), | |
| page_title: name, | |
| page_path: `/${name}`, | |
| page_url: `app://${name}`, | |
| }; | |
| const screenContext: IFormoEventContext = { | |
| page_title: name, | |
| page_path: `/${name}`, | |
| page_url: `app://${name}`, | |
| ...(context ?? {}), | |
| }; |
… events User-provided context values for page_title, page_path, and page_url were being silently discarded because the defaults were spread after the user context. Reversed the spread order so user values take precedence, consistent with how generateContext() handles merging. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tinybird derives page_path from page_url via path(), so setting it in the SDK is redundant and never read. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 participant
Screen events from the React Native SDK now send type="page" instead of type="screen", with screen name mapped to page_title, page_path, and page_url context fields. This allows Tinybird's process_sessions and process_sources materializations (which filter on type='page') to include mobile screen views. The channel="mobile" field distinguishes mobile from web traffic.