Changes : Prepared RevenueCat test builds and refined AI loading UX#59
Changes : Prepared RevenueCat test builds and refined AI loading UX#59aryansoni-dev wants to merge 1 commit into
Conversation
- Isolated Test Store and Google Play configuration by EAS profile
- Hardened RevenueCat identity sync, entitlement checks, request deduplication, and retries
- Added RevenueCat verification scripts, automated tests, and setup documentation
- Added branded sparkle loaders to AI chat and entry reflections
- Improved AI response text wrapping
- Bumped app version and native build numbers to 1.0.9 and 9
📝 WalkthroughWalkthroughThe changes isolate RevenueCat Test Store and Google Play configuration, synchronize subscription identities, harden entitlement and paywall flows, add runtime/configuration tests, introduce a reusable AI processing animation, adjust AI text layout, and update release documentation and metadata. ChangesRevenueCat subscription integration
AI processing and text layout
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Clerk
participant SubscriptionProvider
participant RevenueCatIdentity
participant RevenueCat
participant PaywallScreen
Clerk->>SubscriptionProvider: provide current user
SubscriptionProvider->>RevenueCatIdentity: synchronize identity
RevenueCatIdentity->>RevenueCat: log in or log out
SubscriptionProvider->>RevenueCat: fetch customer info and offerings
RevenueCat-->>PaywallScreen: subscription state and packages
PaywallScreen->>SubscriptionProvider: purchase, restore, or refresh
SubscriptionProvider->>RevenueCat: execute subscription action
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)app.jsonTraceback (most recent call last): eas.jsonTraceback (most recent call last): package.jsonTraceback (most recent call last): 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/ai-chat/ai-chat-screen.tsx (1)
792-806: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAssistant bubble now always renders at full width instead of hugging its content.
Switching the container from
flexShrink/maxWidth/overflow: "visible"to a fixedwidth: assistantBubbleMaxWidth(which equals the full available content width) makes every assistant bubble stretch to fill the row regardless of message length, losing the "shrink-to-content" chat-bubble look for short replies (e.g. the greeting). Sinceai-response-renderer.tsx'ssafeTextStylealready fixes text overflow viaflexShrink/flexWrap/maxWidth: "100%"on theTextitself, the container likely doesn't need a fixed width to avoid clipping.🎨 Suggested fix: restore shrink-to-content sizing
style={{ alignSelf: "flex-start", borderBottomLeftRadius: 24, borderBottomRightRadius: 24, borderCurve: "continuous", borderTopLeftRadius: 24, borderTopRightRadius: 24, boxShadow: "0 3px 8px rgba(39, 39, 42, 0.12)", - width: assistantBubbleMaxWidth, + flexShrink: 1, + maxWidth: assistantBubbleMaxWidth, }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/ai-chat/ai-chat-screen.tsx` around lines 792 - 806, Update the assistant bubble container in the LinearGradient rendering block to restore shrink-to-content sizing instead of using the full-width assistantBubbleMaxWidth. Reuse the prior flexShrink/maxWidth/overflow behavior, while leaving the existing safeTextStyle text overflow handling unchanged.components/paywall/PaywallScreen.tsx (1)
268-282: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPaywall flashes a false "not available" error during normal initial loading.
The
!offering || availablePackages.length === 0branch doesn't checkisLoading. Sinceofferingsstartsnulland only resolves after the identity sync + offerings fetch complete, every paywall open will briefly show "Subscription plans are not available right now." with a disabled "Trying again..." retry button — before the real data arrives. This misrepresents a normal loading state as an error.🐛 Proposed fix to suppress the false error while loading
) : !offering || availablePackages.length === 0 ? ( - <UnavailableMessage - testID="paywall-error-message" - message="Subscription plans are not available right now." - isRetrying={isLoading} - onRetry={() => void refresh()} - /> + isLoading ? null : ( + <UnavailableMessage + testID="paywall-error-message" + message="Subscription plans are not available right now." + isRetrying={isLoading} + onRetry={() => void refresh()} + /> + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/paywall/PaywallScreen.tsx` around lines 268 - 282, Update the conditional rendering around UnavailableMessage so the !offering || availablePackages.length === 0 fallback is suppressed while isLoading is true. Preserve the existing unavailable message and retry behavior once loading completes, as well as the separate error branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/ai-chat/ai-processing-animation.tsx`:
- Around line 51-58: Update the View in the AI processing animation to
communicate its indeterminate loading state to assistive technologies by adding
an appropriate accessibility value or busy accessibility state alongside the
existing accessibilityLabel and progressbar role.
- Around line 87-103: Update the progress assignment in the animation useEffect
to use the React Compiler-compatible shared-value setter instead of direct
.value mutation, while preserving the existing withDelay/withRepeat animation
and cancellation behavior.
- Around line 59-76: Update AiProcessingAnimation and its AnimatedSparkle usages
to generate a unique per-instance SVG ID suffix, and pass it into each sparkle
so all mask, filter, gradient, and url(#...) references use instance-scoped IDs.
Preserve the existing animation directions, delays, and scaling while
eliminating shared literals such as ai-sparkle-center, ai-sparkle-left, and
ai-sparkle-right.
In `@components/ai/ai-response-renderer.tsx`:
- Around line 291-293: Update getRenderedContent in the inline rendering flow so
addSafeBreakOpportunities is not applied to selectable code or link display
text, preserving their exact copyable content while retaining safe wrapping for
ordinary prose. Use the existing parsed-part rendering distinctions to bypass or
replace the transformation for copy-sensitive content, without altering the
underlying link destination.
In `@components/journal-editor/entry-ai-reflection-card.tsx`:
- Around line 336-338: Update the loading branch in ReflectionButton to use a
small loader styled with the button’s white foreground instead of
AiProcessingAnimation’s accent-colored sparkle, ensuring sufficient contrast on
the enabled background. Remove or avoid any nested
accessibilityRole="progressbar" so the loader remains non-announcing within the
button.
In `@lib/subscription/revenueCat.ts`:
- Around line 73-83: Make identityMatches required in hasProEntitlement instead
of defaulting it to true, then update the PaywallScreen call sites to pass true
explicitly with a brief comment that refreshCustomerForUser has already verified
identity; preserve the existing SubscriptionProvider call by supplying its
appropriate explicit identity state.
In `@providers/SubscriptionProvider.tsx`:
- Line 42: Move the active user ID ref assignment out of the render body and
into a useEffect, placing it before the existing identity-sync effect to
preserve ordering. Update the code around activeUserIdRef and clerkUserId only;
leave the synchronizedUserIdRef identityMatches concern unchanged unless needed
for this targeted fix.
In `@tests/revenuecat-runtime.test.ts`:
- Around line 214-225: Remove the tautological journalEntries assertion and its
unused local literal from the expiration test around hasVerifiedEntitlement.
Keep the expiredAccess assertion verifying that an expired entitlement removes
Plus access, and do not imply journal-data isolation unless the test actually
exercises code that can mutate journal data.
---
Outside diff comments:
In `@components/ai-chat/ai-chat-screen.tsx`:
- Around line 792-806: Update the assistant bubble container in the
LinearGradient rendering block to restore shrink-to-content sizing instead of
using the full-width assistantBubbleMaxWidth. Reuse the prior
flexShrink/maxWidth/overflow behavior, while leaving the existing safeTextStyle
text overflow handling unchanged.
In `@components/paywall/PaywallScreen.tsx`:
- Around line 268-282: Update the conditional rendering around
UnavailableMessage so the !offering || availablePackages.length === 0 fallback
is suppressed while isLoading is true. Preserve the existing unavailable message
and retry behavior once loading completes, as well as the separate error branch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4e88675c-bd36-448a-b7b1-1530bea56661
📒 Files selected for processing (26)
app.config.jsapp.jsoncomponents/ai-chat/ai-chat-screen.tsxcomponents/ai-chat/ai-processing-animation.tsxcomponents/ai/ai-response-renderer.tsxcomponents/journal-editor/entry-ai-reflection-card.tsxcomponents/paywall/PaywallScreen.tsxdocs/android-release-dependency-hardening.mddocs/environment-matrix.mddocs/preview-release-readiness.mddocs/revenuecat-preview-runtime-test.mddocs/revenuecat-test-store-setup.mdeas.jsonlib/environment.tslib/subscription/requestDeduper.tslib/subscription/revenueCat.tslib/subscription/revenueCatBuildConfig.jslib/subscription/revenueCatIdentity.tslib/subscription/runOnce.tslib/subscription/subscriptionState.tspackage.jsonproviders/SubscriptionProvider.tsxscripts/verify-revenuecat-config.mjstests/environment.test.tstests/revenuecat-build-config.test.mjstests/revenuecat-runtime.test.ts
💤 Files with no reviewable changes (2)
- tests/environment.test.ts
- lib/environment.ts
| <View | ||
| accessibilityLabel="DearDiary AI is generating a response" | ||
| accessibilityRole="progressbar" | ||
| className="overflow-hidden" | ||
| pointerEvents="none" | ||
| style={{ height: 48 * scale, width: 64 * scale }} | ||
| testID="ai-processing-animation" | ||
| > |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
progressbar role without a value.
accessibilityRole="progressbar" is set without accessibilityValue, so screen readers announce a progress role with no indication of indeterminate progress. Consider adding accessibilityValue={{ now: undefined }} or accessibilityState={{ busy: true }} alongside the label to better communicate an indeterminate loading state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/ai-chat/ai-processing-animation.tsx` around lines 51 - 58, Update
the View in the AI processing animation to communicate its indeterminate loading
state to assistive technologies by adding an appropriate accessibility value or
busy accessibility state alongside the existing accessibilityLabel and
progressbar role.
| <AnimatedSparkle | ||
| delay={0} | ||
| direction="center" | ||
| gradientId="ai-sparkle-center" | ||
| scale={scale} | ||
| /> | ||
| <AnimatedSparkle | ||
| delay={300} | ||
| direction="left" | ||
| gradientId="ai-sparkle-left" | ||
| scale={scale} | ||
| /> | ||
| <AnimatedSparkle | ||
| delay={600} | ||
| direction="right" | ||
| gradientId="ai-sparkle-right" | ||
| scale={scale} | ||
| /> |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Tracked files around ai-processing-animation:"
git ls-files | rg 'ai-processing-animation|ai-chat-screen|entry-ai-reflection-card|package.json|App|components/ai-chat|constants/images' || true
echo
echo "Read target file outline/sections:"
wc -l components/ai-chat/ai-processing-animation.tsx
sed -n '1,240p' components/ai-chat/ai-processing-animation.tsx
echo
echo "Search for AiProcessingAnimation usages:"
rg -n "AiProcessingAnimation|AnimatedSparkle|gradientId=" -S .Repository: aryansoni-dev/dear-diary
Length of output: 9412
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Read reflection card context:"
sed -n '300,355p' components/journal-editor/entry-ai-reflection-card.tsx
echo
echo "Read chat screen context:"
sed -n '560,600p' components/ai-chat/ai-chat-screen.tsx
echo
echo "Check package dependencies for react-native-svg / expo / react-dom:"
if [ -f package.json ]; then jq '.dependencies, .devDependencies' package.json | rg 'react-native-svg|expo|react-dom|next|react'; fi
echo
echo "Read remaining Sparkle implementation:"
sed -n '170,280p' components/ai-chat/ai-processing-animation.tsxRepository: aryansoni-dev/dear-diary
Length of output: 8191
Scope SVG ids per animation instance.
These id literals are shared by every AiProcessingAnimation on screen, while Sparkle resolves url(#...) references from multiple instances against the same IDs. Use a per-instance suffix so each mounted animation defines/uses its own SVG mask, filters, and gradients; this also prevents potential duplicate ID reports.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/ai-chat/ai-processing-animation.tsx` around lines 59 - 76, Update
AiProcessingAnimation and its AnimatedSparkle usages to generate a unique
per-instance SVG ID suffix, and pass it into each sparkle so all mask, filter,
gradient, and url(#...) references use instance-scoped IDs. Preserve the
existing animation directions, delays, and scaling while eliminating shared
literals such as ai-sparkle-center, ai-sparkle-left, and ai-sparkle-right.
| const progress = useSharedValue(0); | ||
|
|
||
| useEffect(() => { | ||
| progress.value = withDelay( | ||
| delay, | ||
| withRepeat( | ||
| withTiming(1, { | ||
| duration: animationDuration, | ||
| easing: Easing.linear, | ||
| }), | ||
| -1, | ||
| false, | ||
| ), | ||
| ); | ||
|
|
||
| return () => cancelAnimation(progress); | ||
| }, [delay, progress]); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Consider .set() over .value for React Compiler compatibility.
Static analysis flags the progress.value = withDelay(...) assignment as incompatible with React Compiler's automatic memoization. Per Reanimated's docs, .get()/.set() are the React Compiler-compliant accessors for shared values. Not an active bug today, but worth adopting if this project intends to enable the React Compiler.
♻️ Suggested change
- progress.value = withDelay(
+ progress.set(withDelay(
delay,
withRepeat(
withTiming(1, {
duration: animationDuration,
easing: Easing.linear,
}),
-1,
false,
),
- );
+ ));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const progress = useSharedValue(0); | |
| useEffect(() => { | |
| progress.value = withDelay( | |
| delay, | |
| withRepeat( | |
| withTiming(1, { | |
| duration: animationDuration, | |
| easing: Easing.linear, | |
| }), | |
| -1, | |
| false, | |
| ), | |
| ); | |
| return () => cancelAnimation(progress); | |
| }, [delay, progress]); | |
| const progress = useSharedValue(0); | |
| useEffect(() => { | |
| progress.set( | |
| withDelay( | |
| delay, | |
| withRepeat( | |
| withTiming(1, { | |
| duration: animationDuration, | |
| easing: Easing.linear, | |
| }), | |
| -1, | |
| false, | |
| ), | |
| ), | |
| ); | |
| return () => cancelAnimation(progress); | |
| }, [delay, progress]); |
🧰 Tools
🪛 React Doctor (0.7.6)
[warning] 90-90: This component misses React Compiler's automatic memoization & re-renders more than it should: This value cannot be modified. Rewrite the flagged code so the compiler can optimize it.
Modifying a value returned from a hook is not allowed. Consider moving the modification into the hook where the value is constructed.
If this is a Reanimated shared value, prefer its React Compiler-compatible .get() / .set() accessors over .value — https://docs.swmansion.com/react-native-reanimated/docs/core/useSharedValue/#react-compiler-support
(immutability)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/ai-chat/ai-processing-animation.tsx` around lines 87 - 103, Update
the progress assignment in the animation useEffect to use the React
Compiler-compatible shared-value setter instead of direct .value mutation, while
preserving the existing withDelay/withRepeat animation and cancellation
behavior.
Source: Linters/SAST tools
| const parts = useMemo(() => parseInlineMarkdown(text), [text]); | ||
| const getRenderedContent = (content: string) => | ||
| selectable ? content : addSafeBreakOpportunities(content); | ||
| addSafeBreakOpportunities(content); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the break-insertion implementation for the character(s) it inserts.
fd 'add-safe-break-opportunities' --extension ts
cat lib/text/add-safe-break-opportunities.tsRepository: aryansoni-dev/dear-diary
Length of output: 1215
🏁 Script executed:
#!/bin/bash
# Inspect how the renderer uses text parts and whether selected/copied display text can include zero-width spaces.
fd 'ai-response-renderer' --extension tsx -x sh -c 'wc -l "$1"; sed -n "260,310p" "$1"; sed -n "380,445p" "$1"' sh {}Repository: aryansoni-dev/dear-diary
Length of output: 2807
Don’t inject break characters into selectable text content.
addSafeBreakOpportunities inserts zero-width spaces into tokens of 32+ non-space characters. Since it now runs for all rendered inline parts, selectable code and long URL display text can be copied with zero-width spaces embedded, while the link still opens with the untouched URL. Keep copy/correction-sensitive content out of this transformation or use a copy-safe wrapping/break approach instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/ai/ai-response-renderer.tsx` around lines 291 - 293, Update
getRenderedContent in the inline rendering flow so addSafeBreakOpportunities is
not applied to selectable code or link display text, preserving their exact
copyable content while retaining safe wrapping for ordinary prose. Use the
existing parsed-part rendering distinctions to bypass or replace the
transformation for copy-sensitive content, without altering the underlying link
destination.
| {isLoading ? ( | ||
| <ActivityIndicator color={disabled ? colors.mutedText : "white"} /> | ||
| <AiProcessingAnimation size="compact" /> | ||
| ) : ( |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Compare brandPrimary vs accent colors, and check the button's original ActivityIndicator color prop history.
rg -n 'brandPrimary|accent' constants/theme.tsRepository: aryansoni-dev/dear-diary
Length of output: 191
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate relevant files =="
fd -a 'entry-ai-reflection-card\.tsx|ai-processing-animation\.tsx|theme\.ts' . | sed 's#^\./##'
echo
echo "== Constants excerpt =="
if [ -f constants/theme.ts ]; then
nl -ba constants/theme.ts | sed -n '1,80p'
fi
echo
echo "== Reflection button excerpt =="
file="$(fd 'entry-ai-reflection-card\.tsx' . | head -n1)"
if [ -n "${file:-}" ]; then
nl -ba "$file" | sed -n '300,365p'
fi
echo
echo "== ai-processing-animation excerpt =="
ans="$(fd 'ai-processing-animation\.tsx' . | head -n1)"
if [ -n "${ans:-}" ]; then
nl -ba "$ans" | sed -n '220,275p'
fi
echo
echo "== Accessibilty role/usages =="
rg -n 'activityindicator|ai processing animation|accessibilityRole|color=' components/journal-editor/entry-ai-reflection-card.tsx components/ai-processing-animation.tsx constants/theme.tsRepository: aryansoni-dev/dear-diary
Length of output: 526
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ref_file="components/journal-editor/entry-ai-reflection-card.tsx"
anim_file="components/ai-chat/ai-processing-animation.tsx"
theme_file="constants/theme.ts"
echo "== constants/theme.ts excerpt =="
sed -n '1,80p' "$theme_file"
echo
echo "== reflection card excerpt =="
sed -n '300,365p' "$ref_file"
echo
echo "== ai-processing-animation excerpt =="
sed -n '220,275p' "$anim_file"
echo
echo "== relevant usages / color props =="
grep -nEi 'activityindicator|ai processing animation|accessibilityRole|accessibilityHint|colors\.(brandPrimary|accent)|color=' "$ref_file" "$anim_file" "$theme_file"Repository: aryansoni-dev/dear-diary
Length of output: 8918
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ref_file="components/journal-editor/entry-ai-reflection-card.tsx"
anim_file="components/ai-chat/ai-processing-animation.tsx"
echo "== imports and first lines of reflection card =="
sed -n '1,70p' "$ref_file"
echo
echo "== imports and processing-animation accessible/root lines =="
sed -n '1,70p' "$anim_file"
echo
echo "== static accessibility tree shapes in the edited file =="
python3 - <<'PY'
from pathlib import Path
p = Path("components/journal-editor/entry-ai-reflection-card.tsx")
s = p.read_text()
for name, content in (("ReflectionButton", s), ("main card top", "\n".join(s.splitlines()[0:140]))):
print(f"--- {name} ---")
for line_num, line in enumerate(content.splitlines(), 1):
if "accessibility" in line or "<Pressable" in line or "<AiProcessingAnimation" in line or "<ActivityIndicator" in line:
print(f"{line_num}: {line}")
PYRepository: aryansoni-dev/dear-diary
Length of output: 5245
Use a contrast-safe, non-accessible loader inside ReflectionButton.
colors.accent is #FF2056, the same color as brandPrimary, so the hardcoded sparkle is not guaranteed to be visible against the enabled button background. ReflectionButton already applies white text for contrast; replace this with a small loading indicator that matches that foreground, and keep it non-announcing by default instead of nesting accessibilityRole="progressbar" inside the accessibilityRole="button" Pressable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/journal-editor/entry-ai-reflection-card.tsx` around lines 336 -
338, Update the loading branch in ReflectionButton to use a small loader styled
with the button’s white foreground instead of AiProcessingAnimation’s
accent-colored sparkle, ensuring sufficient contrast on the enabled background.
Remove or avoid any nested accessibilityRole="progressbar" so the loader remains
non-announcing within the button.
| export function hasProEntitlement( | ||
| customerInfo: CustomerInfo | null, | ||
| identityMatches = true, | ||
| ) { | ||
| return hasVerifiedEntitlement({ | ||
| customerInfo, | ||
| entitlementId: revenueCatEntitlementId, | ||
| identityMatches, | ||
| isAuthoritative: isRevenueCatEntitlementAuthoritative(), | ||
| }); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Defaulting identityMatches to true is a paywall-bypass footgun.
hasProEntitlement's identityMatches = true default means any future/other caller that omits the parameter silently skips identity verification and can grant Pro based on stale/mismatched CustomerInfo. It's safe today only because both current call sites (SubscriptionProvider's memoized isPro, and PaywallScreen after purchasePackage/restorePurchases already verified identity) happen to be safe. Making the parameter required removes the risk of silent misuse later.
♻️ Proposed fix to require explicit identity confirmation
-export function hasProEntitlement(
- customerInfo: CustomerInfo | null,
- identityMatches = true,
-) {
+export function hasProEntitlement(
+ customerInfo: CustomerInfo | null,
+ identityMatches: boolean,
+) {
return hasVerifiedEntitlement({
customerInfo,
entitlementId: revenueCatEntitlementId,
identityMatches,
isAuthoritative: isRevenueCatEntitlementAuthoritative(),
});
}Then update the two PaywallScreen.tsx call sites (lines 106, 137) to pass true explicitly with a short comment explaining why it's safe there (identity already verified by refreshCustomerForUser).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/subscription/revenueCat.ts` around lines 73 - 83, Make identityMatches
required in hasProEntitlement instead of defaulting it to true, then update the
PaywallScreen call sites to pass true explicitly with a brief comment that
refreshCustomerForUser has already verified identity; preserve the existing
SubscriptionProvider call by supplying its appropriate explicit identity state.
| setOfferings(null); | ||
| setError(null); | ||
| }, []); | ||
| activeUserIdRef.current = clerkUserId; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Ref reads/writes during render violate React Compiler purity (flagged by React Doctor).
activeUserIdRef.current = clerkUserId; at line 42 mutates a ref directly in the render body, and identityMatches at lines 332-336 reads synchronizedUserIdRef.current directly in render to feed isPro. Since app.json enables "reactCompiler": true, both are flagged as errors/warnings (no-ref-current-in-render) — React may replay or discard render work, and refs shouldn't be read/written outside effects/handlers. It currently "works" only because every mutation of these refs happens to be paired with an immediate setState call, but that's fragile and incompatible with compiler-driven memoization.
Line 42 has a low-effort fix; line 332-336 would need synchronizedUserIdRef promoted to real state to fully resolve, which is a larger change — flagging both here for awareness, with the immediate ask being line 42.
🔧 Proposed fix for line 42
- activeUserIdRef.current = clerkUserId;
+ useEffect(() => {
+ activeUserIdRef.current = clerkUserId;
+ }, [clerkUserId]);Place this effect before the identity-sync effect so ordering guarantees are preserved.
Also applies to: 332-336
🧰 Tools
🪛 React Doctor (0.7.6)
[error] 42-42: This ref is mutated during render. React can replay or discard render work, so the mutation can leak from UI that never commits.
Move ref writes into an event handler or effect. Render must stay pure because React can replay or discard it. The predictable null-guarded lazy initialization pattern remains supported.
(no-ref-current-in-render)
[warning] 42-42: This component misses React Compiler's automatic memoization & re-renders more than it should: Cannot access refs during render. Rewrite the flagged code so the compiler can optimize it.
React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the current property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef).
(refs)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@providers/SubscriptionProvider.tsx` at line 42, Move the active user ID ref
assignment out of the render body and into a useEffect, placing it before the
existing identity-sync effect to preserve ordering. Update the code around
activeUserIdRef and clerkUserId only; leave the synchronizedUserIdRef
identityMatches concern unchanged unless needed for this targeted fix.
Source: Linters/SAST tools
| const journalEntries = [{ body: "preserved", id: "entry-1" }]; | ||
| const expiredAccess = hasVerifiedEntitlement({ | ||
| customerInfo: customerInfo("user-a", false), | ||
| entitlementId, | ||
| identityMatches: true, | ||
| isAuthoritative: true, | ||
| }); | ||
| assert(!expiredAccess, "An expired entitlement must remove Plus access."); | ||
| assert( | ||
| journalEntries.length === 1 && journalEntries[0]?.body === "preserved", | ||
| "Subscription expiration must not remove core journal data.", | ||
| ); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Tautological assertion doesn't test journal-data isolation.
journalEntries is a local literal never touched by any subscription/entitlement code in this file; asserting its length/body is always true and doesn't verify that subscription expiration actually leaves journal data untouched. This could mislead readers into thinking cross-cutting data safety is covered here.
♻️ Suggested cleanup
- const journalEntries = [{ body: "preserved", id: "entry-1" }];
- const expiredAccess = hasVerifiedEntitlement({
+ const expiredAccess = hasVerifiedEntitlement({
customerInfo: customerInfo("user-a", false),
entitlementId,
identityMatches: true,
isAuthoritative: true,
});
assert(!expiredAccess, "An expired entitlement must remove Plus access.");
- assert(
- journalEntries.length === 1 && journalEntries[0]?.body === "preserved",
- "Subscription expiration must not remove core journal data.",
- );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const journalEntries = [{ body: "preserved", id: "entry-1" }]; | |
| const expiredAccess = hasVerifiedEntitlement({ | |
| customerInfo: customerInfo("user-a", false), | |
| entitlementId, | |
| identityMatches: true, | |
| isAuthoritative: true, | |
| }); | |
| assert(!expiredAccess, "An expired entitlement must remove Plus access."); | |
| assert( | |
| journalEntries.length === 1 && journalEntries[0]?.body === "preserved", | |
| "Subscription expiration must not remove core journal data.", | |
| ); | |
| const expiredAccess = hasVerifiedEntitlement({ | |
| customerInfo: customerInfo("user-a", false), | |
| entitlementId, | |
| identityMatches: true, | |
| isAuthoritative: true, | |
| }); | |
| assert(!expiredAccess, "An expired entitlement must remove Plus access."); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/revenuecat-runtime.test.ts` around lines 214 - 225, Remove the
tautological journalEntries assertion and its unused local literal from the
expiration test around hasVerifiedEntitlement. Keep the expiredAccess assertion
verifying that an expired entitlement removes Plus access, and do not imply
journal-data isolation unless the test actually exercises code that can mutate
journal data.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Release