Skip to content

fix(presentation): give each preload a fresh request id (MOB-652) - #322

Open
kherembourg wants to merge 3 commits into
mainfrom
fix/mob-652-fresh-request-id
Open

kherembourg wants to merge 3 commits into
mainfrom
fix/mob-652-fresh-request-id

Conversation

@kherembourg

@kherembourg kherembourg commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

What

  • PLYPresentationRequest.preload() gives a fresh requestId on each call. display() keeps the current id, and close() / back() target the latest id.
  • PLYPresentationView keeps the requestId it was mounted with, null included. A new request object, or a new preload() of the same request, needs a remount of the view (a new React key).
  • Android bridge: an inline view with no preloaded entry, no placement and no screen now shows nothing and logs a warning. Before, it loaded the app default screen (ply_default). iOS already shows nothing in this case.
  • JSDoc, CHANGELOG.md (Unreleased: Changed + Fixed) and sdk_public_doc.md describe the new requestId rule.

Why

MOB-652. A client (Bayard, RN 6.2.0) saw its inline banner show the full-screen default paywall on Android. One JS request kept one requestId for its whole life, and the native maps used that id as key. An old view unmount, or an old dismiss, deleted the entry of a newer preload() of the same request. The next mount found nothing and the Android bridge loaded ply_default. Production traces on 2026-10-13: a correct placement fetch at 15:33:13 UTC, then GET presentations/ply_default at 15:33:18 UTC, with no CLOSED event.

How to verify

  • cd packages/purchasely && yarn jest src/__tests__/presentation.integration.test.ts src/__tests__/PLYPresentationView.test.tsx -t "MOB-652": 8 passed (head bd25ea9). On the base, 5 of the MOB-652 tests fail because of the shared or re-read id; the others are regression guards. The remount test fails on a variant that caches the id per request object.
  • Both test files without the filter: 90 passed (before the remount test was added).
  • yarn typecheck: clean. npx eslint on the changed source files: no error.
  • Manual (Android): preload() a placement, mount <PLYPresentationView request={request} />, unmount, preload() the same request again, mount a new view: the placement screen shows, never the default screen.

Notes

  • Design chosen by Kevin over per-view ownership tokens on both native maps and callbacks: fresh ids remove the shared key, on Android and iOS, with no native API change.
  • Behavior change: request.requestId changes after each preload(). A mounted view does not follow it; remount the view to show a new preload.
  • Known debt, not fixed here: a preload() that is never mounted, displayed or dismissed keeps its native entry for the life of the process. The native lookups by screen id (clientPresentationDisplayed/Closed, iOS presentation prop) can match an older unused preload of the same screen. This ambiguity already exists today between two request objects of the same screen.
  • The Android empty-target guard has no JVM test: the bridge module has no Robolectric. CI builds it; an E2E or device check proves it.
  • Prettier: the 4 changed files already fail prettier --check on the base (for example, 136 of 172 lines of PLYPresentationView.tsx, 2-space indentation against tabWidth: 4). The new lines follow the surrounding code. ESLint has prettier/prettier off.
  • Review: plan reviewed by Codex gpt-6-astra high (two rounds); final review by a fresh Claude Opus high reviewer on head a8048b8, findings F1 (stale comment) and F2 (subject length) fixed and verified.

🤖 Generated with Claude Code

Greptile

Requested: public SDK contract change, PLYPresentationRequest.requestId now changes on each preload() (packages/purchasely/src/presentation.ts, PLYPresentationView.tsx)

kherembourg and others added 2 commits October 13, 2026 21:23
Each preload() call now assigns a new request id. An old view unmount or
an old dismiss cannot delete the native entry of a newer preload.
PLYPresentationView keeps the request id it mounted with.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When no preloaded entry exists and there is no placement or screen, build
nothing and log a warning. An empty builder loads ply_default.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kherembourg
kherembourg marked this pull request as ready for review October 13, 2026 19:29
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-13T19:32:23.561949Z a8048b8 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@kherembourg

Copy link
Copy Markdown
Contributor Author

@greptile review

@greptile-apps

greptile-apps Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; the added remount test addresses the previous finding.

Summary

Each preload() now gets a fresh requestId. A mounted PLYPresentationView keeps its original id. Android shows nothing when neither a preloaded entry nor a placement or screen is available.

  • Each preload gets its own ID, and views keep the ID they mount with.
  • Android inline views stay blank when they have no target.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A["preload()"] --> B["Assign fresh requestId"]
  B --> C["Mount PLYPresentationView"]
  C --> D["Keep mounted requestId"]
  D --> E["Another preload() gets a new id"]
  E --> F["Change React key"]
  F --> G["Remove old listener and mount with new id"]
Loading

Reviews (2) · Last reviewed commit: "test(presentation): cover a remount afte..." · Reviewed by Greptile

Comment thread packages/purchasely/src/__tests__/PLYPresentationView.test.tsx
A new React key must adopt the id of the latest preload.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kherembourg

Copy link
Copy Markdown
Contributor Author

@greptile review

@kherembourg kherembourg added the run-ci Trigger the E2E / CI build against the current PR head label Oct 13, 2026
@github-actions github-actions Bot removed the run-ci Trigger the E2E / CI build against the current PR head label Oct 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant