Skip to content

SDKS-4824 make iOS cleanup/configure ordering deterministic#30

Merged
pingidentity-gaurav merged 5 commits into
mainfrom
SDKS-4824
Apr 15, 2026
Merged

SDKS-4824 make iOS cleanup/configure ordering deterministic#30
pingidentity-gaurav merged 5 commits into
mainfrom
SDKS-4824

Conversation

@pingidentity-gaurav

@pingidentity-gaurav pingidentity-gaurav commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Fixes iOS race condition where cleanup() could remove a newly registered Journey handle, causing transient "Journey instance not found" errors on rapid dispose → re-init
  • Adds JS integration tests for dispose-then-init and concurrent dispose/init sequencing
  • Adds TSDoc to App.tsx types and component structure

Summary by CodeRabbit

  • New Features

    • Profile-driven initialization and profile selection for startup.
    • Clear user-facing error display when initialization fails.
  • Bug Fixes

    • Improved stability for concurrent init/dispose operations to avoid race conditions.
    • Consistent system font applied to text inputs and labels.
  • Tests

    • Added integration tests covering lifecycle ordering, concurrency, and failure paths.

@github-actions

github-actions Bot commented Apr 8, 2026

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://ForgeRock.github.io/ping-react-native-sdk/docs-preview/pr-30/

Built to branch gh-pages at 2026-04-15 16:03 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@pingidentity-gaurav pingidentity-gaurav changed the title fix(journey): make iOS cleanup/configure ordering deterministic SDKS-4824 make iOS cleanup/configure ordering deterministic Apr 8, 2026

@rodrigoareis rodrigoareis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall changes looks good to me. Left some comments.

Comment thread packages/journey/ios/RNPingJourneyCommon.swift Outdated
Comment thread packages/journey/ios/RNPingJourneyCommon.swift
Comment thread PingTestRunner/__tests__/integration/journey.test.ts
Comment thread PingTestRunner/__tests__/integration/journey.test.ts
Comment thread PingSampleApp/App.tsx Outdated
Comment thread PingTestRunner/__tests__/integration/journey.test.ts

@tsdamas tsdamas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Left minor unblocking comments.

@rodrigoareis rodrigoareis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@coderabbitai

coderabbitai Bot commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Free

Run ID: 31d0c3c4-e4f4-49da-843f-12aa9bd1e423

📥 Commits

Reviewing files that changed from the base of the PR and between 4f8e7af and dea7ae1.

⛔ Files ignored due to path filters (1)
  • .yarn/install-state.gz is excluded by !**/.yarn/**, !**/*.gz
📒 Files selected for processing (16)
  • PingSampleApp/App.tsx
  • PingTestRunner/__tests__/integration/fido.test.ts
  • PingTestRunner/__tests__/integration/journey.test.ts
  • PingTestRunner/__tests__/integration/native-spec-contracts.test.ts
  • PingTestRunner/jest.config.js
  • PingTestRunner/jest.setup.js
  • PingTestRunner/metro.config.js
  • eslint.config.mjs
  • packages/fido/README.md
  • packages/fido/jest.config.js
  • packages/journey/README.md
  • packages/journey/ios/RNPingJourneyCommon.swift
  • packages/journey/src/__tests__/nativeModuleAndProvider.test.tsx
  • packages/journey/src/__tests__/useJourney.test.tsx
  • packages/journey/src/callbackHelpers.ts
  • packages/journey/src/useJourneyForm.ts

📝 Walkthrough

Walkthrough

Adds profile-driven, guarded async initialization and error handling to the React Native sample app; augments journey lifecycle integration tests covering dispose/init concurrency and failure; inlines iOS journey configuration into a serialized lifecycle coordinator block; and includes multiple test/formatting and lint config updates.

Changes

Cohort / File(s) Summary
Sample App
PingSampleApp/App.tsx
Added RootStackParamList and ComponentWithDefaultStyle types; added initError state; moved journey client init into guarded async useEffect that calls selectedJourneyProfile.journeyClient.init() and surfaces init failures by rendering an error Text.
Journey lifecycle (tests)
PingTestRunner/__tests__/integration/journey.test.ts, PingTestRunner/__tests__/integration/fido.test.ts
Added integration tests for journey dispose/init concurrency and failure paths; reformatted several test mocks and expectations (fido test formatting changes only).
iOS native lifecycle
packages/journey/ios/RNPingJourneyCommon.swift
Removed configure() and configureAsync(); updated configureJourney to build the Journey, then call CoreRuntime.setJourneyCallbackResolver(...) and journeyRegistry.register(...) inside a single lifecycleCoordinator.enqueue block; introduced Ref<T> to carry registered journey id and explicit failure handling when registration result is missing.
Journey hook logic
packages/journey/src/useJourneyForm.ts
Refactored internal state sync: added prevFields state and moved hydration from useEffect to a render-time conditional; introduced hasUnsupported flag and reformatted callbacks (useCallback) — internal behavior adjusted for render-time detection.
Journey internals & tests (formatting)
packages/journey/src/callbackHelpers.ts, packages/journey/src/__tests__/*, packages/journey/src/__tests__/nativeModuleAndProvider.test.tsx, packages/journey/src/__tests__/useJourney.test.tsx
Mostly formatting/JSX reflow and added ESLint-disable comments around isolated module requires; one minor trailing-comma signature change in resolveRequiresUserInput.
Test runner config & setup
PingTestRunner/jest.config.js, PingTestRunner/jest.setup.js, PingTestRunner/metro.config.js, eslint.config.mjs
Formatting changes to Jest moduleNameMapper and reporters, expanded mock return object formatting in jest.setup, added ESLint disable in metro config, and added Jest globals block to ESLint config.
Docs & package configs
packages/journey/README.md, packages/fido/README.md, packages/fido/jest.config.js
Formatting and example tweaks: trailing commas, table adjustments, and multi-line signatures/objects; no API type changes beyond formatting.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐇 I nibble bytes and hop through code,

Init and tests along the road.
Swift and JS in tidy cheer,
New configs sprout and logs appear.
Hooray — a rabbit's change is near!


Note

🎁 Summarized by CodeRabbit Free

Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login.

Comment @coderabbitai help to get the list of available commands and usage tips.

@pingidentity-gaurav pingidentity-gaurav merged commit 7483d93 into main Apr 15, 2026
1 of 4 checks passed
@pingidentity-gaurav pingidentity-gaurav deleted the SDKS-4824 branch April 15, 2026 15:57
@codecov

codecov Bot commented Apr 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 69.04762% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.28%. Comparing base (e2a5d1b) to head (dea7ae1).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
packages/journey/src/useJourneyForm.ts 63.63% 6 Missing and 2 partials ⚠️
packages/journey/ios/RNPingJourneyCommon.swift 75.00% 5 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main      #30      +/-   ##
============================================
+ Coverage     57.47%   60.28%   +2.81%     
- Complexity        0       71      +71     
============================================
  Files            16      112      +96     
  Lines           863     9720    +8857     
  Branches        157      376     +219     
============================================
+ Hits            496     5860    +5364     
- Misses          326     3807    +3481     
- Partials         41       53      +12     
Flag Coverage Δ
android 10.87% <ø> (?)
ios 68.88% <75.00%> (?)
javascript 70.88% <63.63%> (+13.41%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants