Skip to content

Added username feature forauthentication#8

Open
Mainak-2006 wants to merge 2 commits into
adrianhajdin:mainfrom
Mainak-2006:added-username-feature-forauthentication
Open

Added username feature forauthentication#8
Mainak-2006 wants to merge 2 commits into
adrianhajdin:mainfrom
Mainak-2006:added-username-feature-forauthentication

Conversation

@Mainak-2006

@Mainak-2006 Mainak-2006 commented May 10, 2026

Copy link
Copy Markdown

Summary by CodeRabbit

Release Notes

  • New Features

    • Added username field to sign-up and option to sign in with email or username.
  • Chores

    • Removed PostHog analytics integration across the application.
    • Simplified app naming and configuration identifiers.
    • Updated user profile display to prioritize username.
    • Refactored app initialization and routing structure.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 10, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR removes PostHog analytics integration across the entire application while enhancing Clerk authentication with username support, updating app identity from "react_native-recurrly" to "recurrly", and refactoring data/UI components for consistency. All PostHog imports, event captures, environment variables, and configuration are eliminated.

Changes

PostHog Analytics Removal & Authentication Enhancement

Layer / File(s) Summary
Analytics Infrastructure Removal
package.json, src/config/posthog.ts
Removes posthog-react-native dependency and deletes entire PostHog client configuration module.
App Configuration & Environment
app.json, .env.example, app.config.js
Updates app identity (name/slug/schemerecurrly), removes PostHog env variables, and adjusts expo.extra composition to preserve only app config extras.
Runtime Data Generation
constants/data.ts
Adds dayjs import; updates HOME_BALANCE.nextRenewalDate and HOME_SUBSCRIPTIONS renewal dates to compute dynamically instead of using static ISO strings.
Authentication with Username
app/(auth)/sign-in.tsx, app/(auth)/sign-up.tsx
Refactors sign-in to use signIn.create({identifier, password}) accepting email or username; adds username field to sign-up with separate validation state; removes all PostHog event tracking.
Root Layout & Navigation
app/_layout.tsx
Removes PostHogProvider wrapper and screen-autocapture logic; simplifies RootLayoutContent to gate on fontsLoaded and authLoaded only; replaces unconfigured stack with explicit Stack.Screen entries for (auth), (tabs), onboarding, and subscriptions/[id].
Screen-Level Integration
app/(tabs)/index.tsx, app/(tabs)/settings.tsx, app/subscriptions/[id].tsx
Removes usePostHog hooks and all capture/identify calls; updates displayName to prefer user.username, then user.fullName, then email; rewrites upcoming subscription filter to use dayjs().startOf("day") and 0–7 days inclusivity; adds explicit daysLeft prop.
Modal Component Refactoring
components/CreateSubscriptionModal.tsx
Introduces isValidForm derived state, extracts resetForm() helper, calls it before closing modal, and refactors Frequency/Category UI to compute className via clsx(...) instead of inline ternary expressions.
Documentation & Cleanup
README.md, lib/subscriptionStore.ts
Removes PostHog badge and "Production-Grade Analytics" feature entry; expands Quick Start with Expo initialization and Expo Go setup instructions; updates .env example to include only Clerk key; adds minor formatting.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

Possibly related PRs

Poem

🐰 PostHog no more, we say goodbye,
Clerk's username makes users fly,
With dayjs dates and forms so clean,
The recurrly app, a sleeker scene!
No analytics haunting our code,
Just auth and grace down the road. 🌿

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title claims to add a username feature for authentication, but the changes include extensive PostHog analytics removal, app identity renaming, Clerk integration updates, and environment configuration changes—making the title incomplete and misleading about the full scope. Revise the title to accurately reflect the primary changes, such as 'Remove PostHog analytics and add username-based authentication' or 'Refactor auth with username support and remove analytics integration'.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 7

🧹 Nitpick comments (8)
app/_layout.tsx (1)

39-44: 💤 Low value

Consider: Redundant screenOptions on Stack.

Line 39 sets screenOptions={{ headerShown: false }} on the Stack, but each child Stack.Screen also explicitly sets options={{ headerShown: false }}. The child options are redundant since they inherit from the parent's screenOptions.

♻️ Optional simplification
  return (
    <Stack screenOptions={{ headerShown: false }}>
-     <Stack.Screen name="(auth)" options={{ headerShown: false }} />
-     <Stack.Screen name="(tabs)" options={{ headerShown: false }} />
-     <Stack.Screen name="onboarding" options={{ headerShown: false }} />
-     <Stack.Screen name="subscriptions/[id]" options={{ headerShown: false }} />
+     <Stack.Screen name="(auth)" />
+     <Stack.Screen name="(tabs)" />
+     <Stack.Screen name="onboarding" />
+     <Stack.Screen name="subscriptions/[id]" />
    </Stack>
  );
🤖 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 `@app/_layout.tsx` around lines 39 - 44, The Stack-level screenOptions already
sets headerShown: false, so remove the redundant options={{ headerShown: false
}} from the individual Stack.Screen entries (e.g., the Stack.Screen instances
for "(auth)", "(tabs)", "onboarding", and "subscriptions/[id]") so they inherit
the parent setting; alternatively, if any child needs different header behavior,
keep that child's options and remove the parent screenOptions—update the
Stack/Stack.Screen usage accordingly.
constants/data.ts (2)

17-17: 💤 Low value

Note: Date computed at module load time.

The dayjs() call executes when the module loads, not on each render. This means nextRenewalDate will be fixed to "3 days from app start" until the app restarts or the module hot-reloads. If the app runs for extended periods, this date won't update.

If you need the date to update dynamically, consider computing it in a component or using a function that returns the current value.

🤖 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 `@constants/data.ts` at line 17, The constant nextRenewalDate is computed at
module load time causing a stale value; change it to a function or getter (e.g.,
getNextRenewalDate or nextRenewalDate() function) that returns dayjs().add(3,
"day").format("YYYY-MM-DD") so callers compute the date on demand (or move the
computation into the component where it's used); update all usages of
nextRenewalDate to call the new function/getter instead of reading a static
value.

60-105: ⚡ Quick win

Inconsistent renewal date handling across subscription statuses.

Lines 60 and 75 use dynamic dates via dayjs().add(N, "day").toISOString() for active subscriptions, while lines 90 and 105 use hardcoded future dates ("2026-03-27T10:00:00.000Z", "2026-04-02T10:00:00.000Z") for paused/cancelled subscriptions.

This inconsistency could cause confusion. Consider either:

  1. Using dynamic dates for all subscriptions regardless of status, or
  2. Adding a comment explaining why paused/cancelled subscriptions use fixed future dates
🤖 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 `@constants/data.ts` around lines 60 - 105, The renewalDate values are
inconsistent: active items use dynamic dayjs().add(...).toISOString() while
paused/cancelled items (ids "claude-pro" and "canva-pro") use hardcoded ISO
strings; either make them consistent by replacing the hardcoded renewalDate
values for the objects with id "claude-pro" and "canva-pro" to use the same
dayjs().add(...).toISOString() pattern (matching how renewalDate is set on
active entries) or, if fixed dates are intentional, add an inline comment next
to the renewalDate fields explaining why those specific ISO strings are required
so the intent is clear when reading the renewalDate property.
app/(auth)/sign-in.tsx (1)

51-72: ⚡ Quick win

Extract the duplicated signIn.finalize navigate logic.

The post-finalize navigation block in handleSubmit (Lines 51–72) and handleVerify (Lines 93–114) is identical. Pull it into a single helper to avoid drift if the navigation logic changes.

♻️ Suggested helper
+  const navigateAfterFinalize = ({ session, decorateUrl }: any) => {
+    if (session?.currentTask) {
+      console.log(session?.currentTask);
+      return;
+    }
+    const url = decorateUrl("/(tabs)");
+    if (url.startsWith("http")) {
+      if (typeof window !== "undefined" && window.location) {
+        window.location.href = url;
+      } else {
+        router.replace("/(tabs)" as Href);
+      }
+    } else {
+      router.replace(url as Href);
+    }
+  };

Then use await signIn.finalize({ navigate: navigateAfterFinalize }) in both handlers.

Also applies to: 93-114

🤖 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 `@app/`(auth)/sign-in.tsx around lines 51 - 72, The duplicate post-finalize
navigation logic should be extracted into a single helper (e.g.,
navigateAfterFinalize) and reused in both handleSubmit and handleVerify: create
a function (navigateAfterFinalize) that accepts the same ({ session, decorateUrl
}) signature used by signIn.finalize and implements the existing logic (check
session?.currentTask, compute url via decorateUrl("/(tabs)"), handle absolute
URLs via window.location.href or router.replace, otherwise router.replace(url)),
then replace both await signIn.finalize({ navigate: ... }) calls with await
signIn.finalize({ navigate: navigateAfterFinalize }) to avoid duplication and
drift.
app/(auth)/sign-up.tsx (3)

67-91: ⚡ Quick win

Same finalize-navigate block duplicated across sign-in and sign-up.

This navigate callback is identical to the one in app/(auth)/sign-in.tsx (in two places there as well). Extracting it into a shared helper (e.g. lib/auth/navigateAfterFinalize.ts) would prevent the three copies from drifting.

🤖 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 `@app/`(auth)/sign-up.tsx around lines 67 - 91, The navigate callback passed to
signUp.finalize is duplicated across sign-up and sign-in; extract this logic
into a single helper (e.g. export a function navigateAfterFinalize(decorateUrl,
router) or navigateAfterFinalize({decorateUrl, router, session}) in
lib/auth/navigateAfterFinalize.ts) and replace the inline navigate blocks in
signUp.finalize and signIn.finalize with a reference to that helper, ensuring it
performs the same checks (session?.currentTask, decorateUrl("/(tabs)"),
window.location.href for http URLs, and router.replace for native/non-http URLs)
and maintains types for Href/router usage.

23-25: ⚡ Quick win

Rename userNameusername for consistency.

The setter is setUsername and Clerk's field is username, but the state variable is userName. Aligning the names removes a small but easily-confused inconsistency throughout the file (validation, JSX, submit payload).

♻️ Proposed rename
-  const [userName, setUsername] = useState("");
+  const [username, setUsername] = useState("");

…and update references (usernameValid, userName.length, value={userName}, username: userNameusername).

🤖 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 `@app/`(auth)/sign-up.tsx around lines 23 - 25, Rename the state variable
userName to username to match the setter setUsername and Clerk's field; update
all references including validations (usernameValid), length checks
(userName.length → username.length), JSX props (value={userName} →
value={username}) and the submit payload (username: userName → username). Ensure
the const declaration becomes const [username, setUsername] = useState("") and
run a quick search in this file to replace any remaining userName identifiers
with username so names are consistent throughout sign-up logic and UI.

42-60: ⚡ Quick win

Dead if (!error) branch after early return; also handle sendEmailCode failures.

You already early-return when error is truthy (Lines 51–54), so the if (!error) check on Line 57 is always true and can be dropped. Additionally, signUp.verifications.sendEmailCode() is awaited but unguarded — if it rejects, the user lands in an inconsistent state with no feedback.

♻️ Proposed cleanup
   const handleSubmit = async () => {
     if (!formValid) return;

     const { error } = await signUp.password({
       emailAddress,
       username: userName,
       password,
     });

     if (error) {
       console.error(JSON.stringify(error, null, 2));
       return;
     }

-    // Send verification email
-    if (!error) {
-      await signUp.verifications.sendEmailCode();
-    }
+    // Send verification email
+    try {
+      await signUp.verifications.sendEmailCode();
+    } catch (e) {
+      console.error("Failed to send verification email:", e);
+    }
   };
🤖 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 `@app/`(auth)/sign-up.tsx around lines 42 - 60, The handleSubmit function
contains a dead `if (!error)` after an early return from the `signUp.password`
error case; remove that redundant conditional and call
`signUp.verifications.sendEmailCode()` directly. Wrap the
`signUp.verifications.sendEmailCode()` call in a try/catch (inside handleSubmit)
so any rejection is handled (log the error and surface user feedback or set an
error state and return), and ensure you don’t assume success after
`signUp.password`—use the caught error to keep the UI consistent.
app/(tabs)/index.tsx (1)

44-49: ⚡ Quick win

Remove unused isExpanding variable.

isExpanding is computed but never referenced — looks like a leftover from the removed PostHog capture call. Drop it to keep the handler clean.

♻️ Proposed cleanup
   const handleSubscriptionPress = (item: Subscription) => {
-    const isExpanding = expandedSubscriptionId !== item.id;
     setExpandedSubscriptionId((currentId) =>
       currentId === item.id ? null : item.id,
     );
   };
🤖 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 `@app/`(tabs)/index.tsx around lines 44 - 49, The handler
handleSubscriptionPress computes an unused variable isExpanding; remove the
isExpanding declaration and any related dead code so the function only updates
expandedSubscriptionId (using setExpandedSubscriptionId and the Subscription
item.id), leaving the rest of the logic intact; reference:
handleSubscriptionPress, isExpanding, expandedSubscriptionId, Subscription.
🤖 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 `@app.json`:
- Around line 3-4: package.json's "name" value is inconsistent with app.json;
open package.json and change the "name" field from "react_native-recurrly" to
"recurrly" so it matches app.json, search for any other occurrences of the old
name and update them, then run npm install (or yarn) to regenerate
package-lock.json and ensure the project builds with the updated app name.

In `@app/`(auth)/sign-in.tsx:
- Around line 30-44: The username check (isUsername) and identifier handling are
too permissive and don't trim whitespace; update validation to use a username
regex matching Clerk's constraints (e.g. /^[A-Za-z0-9_-]{4,64}$/) and compute
all validations against a trimmedIdentifier (const trimmedIdentifier =
identifier.trim()); replace uses of isUsername, isEmail and identifierValid to
reference trimmedIdentifier, and update handleSubmit to call signIn.create with
the trimmedIdentifier and password (e.g., pass identifier: trimmedIdentifier) so
leading/trailing whitespace is removed before sending to signIn.create.

In `@app/`(tabs)/index.tsx:
- Around line 55-60: Update the stale comment above the displayName computation
to accurately describe the new fallback order: username, fullName, or email (and
finally "User"); locate the comment immediately preceding the const displayName
declaration (references: displayName, user) and change the text from "Get user
display name: firstName, fullName, or email" to something like "Get user display
name: username, fullName, or email" so it matches the code's fallback chain.
- Around line 99-106: The daysLeft passed into UpcomingSubscriptionCard is
computed with millisecond precision and differs from the filter's day-aligned
calculation; update the renderItem to compute daysLeft exactly like the filter
by using dayjs(item.renewalDate).startOf("day").diff(dayjs().startOf("day"),
"day") so the value used in FlatList renderItem (where UpcomingSubscriptionCard
is instantiated) matches the days-based filter applied to upcomingSubscriptions.

In `@README.md`:
- Around line 112-130: Update the Quick Start section in README to separate two
clear workflows: "Create a new Expo app from scratch" (show the npx
create-expo-app and --template examples) and a distinct "Using this cloned
repository" subsection that explains to run npm/yarn/pnpm install and start the
dev server; remove or reword the confusing sentence that mixes both flows and
ensure each subsection has its own header and the appropriate commands so users
who cloned the repo are not shown the create-app steps as the next action.
- Line 167: The README incorrectly directs users to "Expo" for credentials while
the env var is EXPO_PUBLIC_CLERK_PUBLISHABLE_KEY (a Clerk credential); update
the text to reference Clerk instead of Expo and point the signup/link to Clerk's
signup or documentation (e.g., Clerk signup URL) so the instruction matches the
EXPO_PUBLIC_CLERK_PUBLISHABLE_KEY variable name.

---

Nitpick comments:
In `@app/_layout.tsx`:
- Around line 39-44: The Stack-level screenOptions already sets headerShown:
false, so remove the redundant options={{ headerShown: false }} from the
individual Stack.Screen entries (e.g., the Stack.Screen instances for "(auth)",
"(tabs)", "onboarding", and "subscriptions/[id]") so they inherit the parent
setting; alternatively, if any child needs different header behavior, keep that
child's options and remove the parent screenOptions—update the
Stack/Stack.Screen usage accordingly.

In `@app/`(auth)/sign-in.tsx:
- Around line 51-72: The duplicate post-finalize navigation logic should be
extracted into a single helper (e.g., navigateAfterFinalize) and reused in both
handleSubmit and handleVerify: create a function (navigateAfterFinalize) that
accepts the same ({ session, decorateUrl }) signature used by signIn.finalize
and implements the existing logic (check session?.currentTask, compute url via
decorateUrl("/(tabs)"), handle absolute URLs via window.location.href or
router.replace, otherwise router.replace(url)), then replace both await
signIn.finalize({ navigate: ... }) calls with await signIn.finalize({ navigate:
navigateAfterFinalize }) to avoid duplication and drift.

In `@app/`(auth)/sign-up.tsx:
- Around line 67-91: The navigate callback passed to signUp.finalize is
duplicated across sign-up and sign-in; extract this logic into a single helper
(e.g. export a function navigateAfterFinalize(decorateUrl, router) or
navigateAfterFinalize({decorateUrl, router, session}) in
lib/auth/navigateAfterFinalize.ts) and replace the inline navigate blocks in
signUp.finalize and signIn.finalize with a reference to that helper, ensuring it
performs the same checks (session?.currentTask, decorateUrl("/(tabs)"),
window.location.href for http URLs, and router.replace for native/non-http URLs)
and maintains types for Href/router usage.
- Around line 23-25: Rename the state variable userName to username to match the
setter setUsername and Clerk's field; update all references including
validations (usernameValid), length checks (userName.length → username.length),
JSX props (value={userName} → value={username}) and the submit payload
(username: userName → username). Ensure the const declaration becomes const
[username, setUsername] = useState("") and run a quick search in this file to
replace any remaining userName identifiers with username so names are consistent
throughout sign-up logic and UI.
- Around line 42-60: The handleSubmit function contains a dead `if (!error)`
after an early return from the `signUp.password` error case; remove that
redundant conditional and call `signUp.verifications.sendEmailCode()` directly.
Wrap the `signUp.verifications.sendEmailCode()` call in a try/catch (inside
handleSubmit) so any rejection is handled (log the error and surface user
feedback or set an error state and return), and ensure you don’t assume success
after `signUp.password`—use the caught error to keep the UI consistent.

In `@app/`(tabs)/index.tsx:
- Around line 44-49: The handler handleSubscriptionPress computes an unused
variable isExpanding; remove the isExpanding declaration and any related dead
code so the function only updates expandedSubscriptionId (using
setExpandedSubscriptionId and the Subscription item.id), leaving the rest of the
logic intact; reference: handleSubscriptionPress, isExpanding,
expandedSubscriptionId, Subscription.

In `@constants/data.ts`:
- Line 17: The constant nextRenewalDate is computed at module load time causing
a stale value; change it to a function or getter (e.g., getNextRenewalDate or
nextRenewalDate() function) that returns dayjs().add(3,
"day").format("YYYY-MM-DD") so callers compute the date on demand (or move the
computation into the component where it's used); update all usages of
nextRenewalDate to call the new function/getter instead of reading a static
value.
- Around line 60-105: The renewalDate values are inconsistent: active items use
dynamic dayjs().add(...).toISOString() while paused/cancelled items (ids
"claude-pro" and "canva-pro") use hardcoded ISO strings; either make them
consistent by replacing the hardcoded renewalDate values for the objects with id
"claude-pro" and "canva-pro" to use the same dayjs().add(...).toISOString()
pattern (matching how renewalDate is set on active entries) or, if fixed dates
are intentional, add an inline comment next to the renewalDate fields explaining
why those specific ISO strings are required so the intent is clear when reading
the renewalDate property.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 15e871b6-cba8-48a0-9146-36891324f97d

📥 Commits

Reviewing files that changed from the base of the PR and between 8f2eb15 and 8cae0ce.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (15)
  • .env.example
  • README.md
  • app.config.js
  • app.json
  • app/(auth)/sign-in.tsx
  • app/(auth)/sign-up.tsx
  • app/(tabs)/index.tsx
  • app/(tabs)/settings.tsx
  • app/_layout.tsx
  • app/subscriptions/[id].tsx
  • components/CreateSubscriptionModal.tsx
  • constants/data.ts
  • lib/subscriptionStore.ts
  • package.json
  • src/config/posthog.ts
💤 Files with no reviewable changes (3)
  • .env.example
  • package.json
  • src/config/posthog.ts

Comment thread app.json
Comment on lines +3 to +4
"name": "recurrly",
"slug": "recurrly",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check for references to the old app name in configuration
rg -i 'react_native-recurrly' -g '!app.json' -C2

Repository: adrianhajdin/react-native-recurrly

Length of output: 586


Update package.json to match the app name change in app.json.

The app name was changed to recurrly in app.json, but package.json still references react_native-recurrly. This inconsistency can cause build and deployment issues in Expo projects. Update the name field in package.json to recurrly to maintain consistency across your build configuration. The package-lock.json will be regenerated automatically after this change.

🤖 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 `@app.json` around lines 3 - 4, package.json's "name" value is inconsistent
with app.json; open package.json and change the "name" field from
"react_native-recurrly" to "recurrly" so it matches app.json, search for any
other occurrences of the old name and update them, then run npm install (or
yarn) to regenerate package-lock.json and ensure the project builds with the
updated app name.

Comment thread app/(auth)/sign-in.tsx
Comment on lines +30 to +44
// Client-side validation: accepts email or username
const isEmail = /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(identifier);
const isUsername = identifier.length > 0 && !identifier.includes("@");
const identifierValid = identifier.length === 0 || isEmail || isUsername;
const passwordValid = password.length > 0;
const formValid =
identifier.length > 0 && password.length > 0 && identifierValid;

const handleSubmit = async () => {
if (!formValid) return;
const handleSubmit = async () => {
if (!formValid) return;

const { error } = await signIn.password({
emailAddress,
password,
});
const { error } = await signIn.create({
identifier,
password,
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

What are the default username constraints (allowed characters, min/max length) for Clerk @clerk/expo v3 useSignIn().signIn.create({identifier, password})?

💡 Result:

Default Clerk @clerk/expo v3 useSignIn().signIn.create({ identifier, password }) username/identifier constraints are not defined on the useSignIn() / signIn.create() API page itself; however, when Clerk usernames are enabled, the documented default username rules are: 1) Allowed characters / character sets - Usernames support Latin-based characters only. [1] - Usernames do not allow special characters: ^$!.#+~. [2][1] 2) Min / max length - Usernames must be between 4 and 64 characters long. [2][1] Notes about scope - The signIn.create()call uses anidentifier` field that can be email, phone, or username depending on how your Clerk instance is configured. The min/max/special-character rules above apply to the username identifier specifically (not to email/phone identifiers). [3] Password - This question asked about “username constraints”; Clerk’s password rules (when passwords are enabled) are described separately (e.g., minimum 8 characters for subscriber-chosen secrets). [4]

Citations:


🌐 Web query:

Clerk username allowed characters alphanumeric underscore hyphen period special characters documentation

💡 Result:

Clerk usernames must be between 4 and 64 characters long.[1][2] They do not allow special characters (^ $!. ` # + ~), implying alphanumeric characters plus underscore and hyphen are permitted, as these are standard for usernames and not listed as prohibited.[1][2] Usernames only support Latin-based characters to protect against Unicode spoofing and homograph attacks.[1][2] These rules can be customized in the Clerk Dashboard.[1][2]

Citations:


Username validation is ineffective and identifier needs trimming before submit.

isUsername = identifier.length > 0 && !identifier.includes("@") accepts any non-empty string without an @ — including whitespace-only input, leading/trailing spaces, or arbitrary symbols. This makes identifierValid true for almost any input, leaving only a "non-empty" check. Additionally, trim the identifier before submitting to avoid Clerk rejections from accidental whitespace.

For stricter client-side validation, match Clerk's actual username constraints: alphanumeric characters plus underscore and hyphen only, 4–64 characters long.

🛡️ Suggested change
-  // Client-side validation: accepts email or username
-  const isEmail = /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(identifier);
-  const isUsername = identifier.length > 0 && !identifier.includes("@");
-  const identifierValid = identifier.length === 0 || isEmail || isUsername;
+  // Client-side validation: accepts email or username
+  const trimmedIdentifier = identifier.trim();
+  const isEmail = /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(trimmedIdentifier);
+  // Match Clerk username constraints: alphanumeric, underscore, hyphen; 4–64 chars.
+  const isUsername = /^[A-Za-z0-9_-]{4,64}$/.test(trimmedIdentifier);
+  const identifierValid = trimmedIdentifier.length === 0 || isEmail || isUsername;
   const passwordValid = password.length > 0;
   const formValid =
-    identifier.length > 0 && password.length > 0 && identifierValid;
+    trimmedIdentifier.length > 0 && password.length > 0 && identifierValid;

   const handleSubmit = async () => {
     if (!formValid) return;

     const { error } = await signIn.create({
-      identifier,
+      identifier: trimmedIdentifier,
       password,
     });
📝 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.

Suggested change
// Client-side validation: accepts email or username
const isEmail = /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(identifier);
const isUsername = identifier.length > 0 && !identifier.includes("@");
const identifierValid = identifier.length === 0 || isEmail || isUsername;
const passwordValid = password.length > 0;
const formValid =
identifier.length > 0 && password.length > 0 && identifierValid;
const handleSubmit = async () => {
if (!formValid) return;
const handleSubmit = async () => {
if (!formValid) return;
const { error } = await signIn.password({
emailAddress,
password,
});
const { error } = await signIn.create({
identifier,
password,
});
// Client-side validation: accepts email or username
const trimmedIdentifier = identifier.trim();
const isEmail = /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(trimmedIdentifier);
// Match Clerk username constraints: alphanumeric, underscore, hyphen; 4–64 chars.
const isUsername = /^[A-Za-z0-9_-]{4,64}$/.test(trimmedIdentifier);
const identifierValid = trimmedIdentifier.length === 0 || isEmail || isUsername;
const passwordValid = password.length > 0;
const formValid =
trimmedIdentifier.length > 0 && password.length > 0 && identifierValid;
const handleSubmit = async () => {
if (!formValid) return;
const { error } = await signIn.create({
identifier: trimmedIdentifier,
password,
});
🤖 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 `@app/`(auth)/sign-in.tsx around lines 30 - 44, The username check (isUsername)
and identifier handling are too permissive and don't trim whitespace; update
validation to use a username regex matching Clerk's constraints (e.g.
/^[A-Za-z0-9_-]{4,64}$/) and compute all validations against a trimmedIdentifier
(const trimmedIdentifier = identifier.trim()); replace uses of isUsername,
isEmail and identifierValid to reference trimmedIdentifier, and update
handleSubmit to call signIn.create with the trimmedIdentifier and password
(e.g., pass identifier: trimmedIdentifier) so leading/trailing whitespace is
removed before sending to signIn.create.

Comment thread app/(tabs)/index.tsx
Comment on lines +55 to +60
// Get user display name: firstName, fullName, or email
const displayName =
user?.username ||
user?.fullName ||
user?.emailAddresses[0]?.emailAddress ||
"User";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update stale comment to match the new fallback chain.

The comment still says firstName, fullName, or email, but the code now reads username first. Fix the comment to avoid confusing future readers.

📝 Proposed fix
-  // Get user display name: firstName, fullName, or email
+  // Get user display name: username, fullName, or email
   const displayName =
     user?.username ||
     user?.fullName ||
     user?.emailAddresses[0]?.emailAddress ||
     "User";
📝 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.

Suggested change
// Get user display name: firstName, fullName, or email
const displayName =
user?.username ||
user?.fullName ||
user?.emailAddresses[0]?.emailAddress ||
"User";
// Get user display name: username, fullName, or email
const displayName =
user?.username ||
user?.fullName ||
user?.emailAddresses[0]?.emailAddress ||
"User";
🤖 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 `@app/`(tabs)/index.tsx around lines 55 - 60, Update the stale comment above
the displayName computation to accurately describe the new fallback order:
username, fullName, or email (and finally "User"); locate the comment
immediately preceding the const displayName declaration (references:
displayName, user) and change the text from "Get user display name: firstName,
fullName, or email" to something like "Get user display name: username,
fullName, or email" so it matches the code's fallback chain.

Comment thread app/(tabs)/index.tsx
Comment on lines +99 to +106
<FlatList
data={upcomingSubscriptions}
renderItem={({ item }) => (
<UpcomingSubscriptionCard
{...item}
daysLeft={dayjs(item.renewalDate).diff(dayjs(), "day")}
/>
)}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

daysLeft passed to card is inconsistent with the filter.

The filter on Lines 29–42 computes daysLeft against dayjs().startOf("day") and dayjs(sub.renewalDate).startOf("day"), but the prop here uses dayjs(item.renewalDate).diff(dayjs(), "day") (millisecond-precision, truncated toward zero). As a result, the card can display a different number than the filter used to include it (e.g. a renewal 6 days 23 hours away is included as daysLeft = 7 by the filter but rendered as 6; a renewal earlier today renders as -1). Reuse the same day-aligned computation so the UI matches the filter.

🐛 Proposed fix
               <FlatList
                 data={upcomingSubscriptions}
                 renderItem={({ item }) => (
                   <UpcomingSubscriptionCard
                     {...item}
-                    daysLeft={dayjs(item.renewalDate).diff(dayjs(), "day")}
+                    daysLeft={dayjs(item.renewalDate)
+                      .startOf("day")
+                      .diff(dayjs().startOf("day"), "day")}
                   />
                 )}
📝 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.

Suggested change
<FlatList
data={upcomingSubscriptions}
renderItem={({ item }) => (
<UpcomingSubscriptionCard
{...item}
daysLeft={dayjs(item.renewalDate).diff(dayjs(), "day")}
/>
)}
<FlatList
data={upcomingSubscriptions}
renderItem={({ item }) => (
<UpcomingSubscriptionCard
{...item}
daysLeft={dayjs(item.renewalDate)
.startOf("day")
.diff(dayjs().startOf("day"), "day")}
/>
)}
🤖 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 `@app/`(tabs)/index.tsx around lines 99 - 106, The daysLeft passed into
UpcomingSubscriptionCard is computed with millisecond precision and differs from
the filter's day-aligned calculation; update the renderItem to compute daysLeft
exactly like the filter by using
dayjs(item.renewalDate).startOf("day").diff(dayjs().startOf("day"), "day") so
the value used in FlatList renderItem (where UpcomingSubscriptionCard is
instantiated) matches the days-based filter applied to upcomingSubscriptions.

Comment on lines 4 to +13
const SubscriptionDetails = () => {
const { id } = useLocalSearchParams<{ id: string }>();
const posthog = usePostHog();
const { id } = useLocalSearchParams<{ id: string }>();

useEffect(() => {
// Only capture if id is valid
if (id && typeof id === 'string' && id.trim()) {
posthog.capture('subscription_details_viewed', { subscription_id: id });
}
}, [id, posthog]);
return (
<View>
<Text>Subscription Details: {id}</Text>
<Link href="/">Go back</Link>
</View>
);
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if this route is actually used/linked from anywhere
rg -n "subscriptions/\[id\]|router\.push.*subscriptions" -g '!subscriptions/[id].tsx' -C2

Repository: adrianhajdin/react-native-recurrly

Length of output: 417


🏁 Script executed:

# 1. Search for navigations to subscriptions more broadly
rg -n "subscriptions" --type tsx --type ts --type jsx --type js -i | head -30

Repository: adrianhajdin/react-native-recurrly

Length of output: 106


🏁 Script executed:

# 2. List all screens in the app to understand the structure
fd -t f '\.(tsx|ts)$' app/ | grep -E '(layout|\[|index)' | head -20

Repository: adrianhajdin/react-native-recurrly

Length of output: 188


🏁 Script executed:

# 3. Look for other detail screens with [id] pattern to see the implementation standard
fd -t f '\[id\]\.tsx$' app/

Repository: adrianhajdin/react-native-recurrly

Length of output: 105


🏁 Script executed:

# 4. Check the current component for any TODO/FIXME comments
cat -n app/subscriptions/\[id\].tsx

Repository: adrianhajdin/react-native-recurrly

Length of output: 540


🏁 Script executed:

# Search for subscriptions references without type filter
rg "subscriptions" -i --max-count 20

Repository: adrianhajdin/react-native-recurrly

Length of output: 3236


🏁 Script executed:

# Check if there's a subscriptions directory/list
ls -la app/subscriptions/ 2>/dev/null || echo "Directory check"

Repository: adrianhajdin/react-native-recurrly

Length of output: 244


🏁 Script executed:

# Examine tabs screens to see implementation patterns
cat -n app/\(tabs\)/index.tsx | head -50

Repository: adrianhajdin/react-native-recurrly

Length of output: 2409


🏁 Script executed:

# Check the _layout.tsx to see if subscriptions route is conditional or always shown
cat -n app/_layout.tsx

Repository: adrianhajdin/react-native-recurrly

Length of output: 2330


🏁 Script executed:

# Check SubscriptionCard component for navigation
rg -n "subscriptions/\[id\]|href.*subscriptions|router.*push" app/components/ -A 3 -B 1

Repository: adrianhajdin/react-native-recurrly

Length of output: 134


🏁 Script executed:

# Find where components directory is
fd -t d components

Repository: adrianhajdin/react-native-recurrly

Length of output: 90


🏁 Script executed:

# Also check the SubscriptionCard import to see where it comes from
rg -n "SubscriptionCard" | head -5

Repository: adrianhajdin/react-native-recurrly

Length of output: 666


🏁 Script executed:

# Check SubscriptionCard implementation
cat -n components/SubscriptionCard.tsx

Repository: adrianhajdin/react-native-recurrly

Length of output: 4364


🏁 Script executed:

# Check if there are any Link or router.push usages in components
rg -n "href|router\.push|Link" components/ | grep -i "subscription"

Repository: adrianhajdin/react-native-recurrly

Length of output: 60


Subscription details route is incomplete and unreachable.

The component contains only a placeholder with ID text and back link. Additionally, no navigation in the app points to this route—the subscriptions/[id] route is registered but never actually accessed. The SubscriptionCard component already displays all subscription details inline when expanded, making the purpose of this detail screen unclear.

Either implement this screen properly (with subscription data fetching, SafeAreaView, and styled layout consistent with other screens) and add navigation to it, or remove it if no longer needed.

Add a TODO comment to track the decision:

// TODO: Implement full subscription details screen or remove if using inline expansion instead

Comment thread README.md
Comment on lines +112 to +130
Before you begin developing your app, it's essential to set up Expo and initialize your project.

```bash
npx create-expo-app@latest
```
- Start by creating a folder for your application. Then, open your terminal and navigate to that folder. To create an Expo app, you can use:

By default, you get a TypeScript Expo app with Expo Router configured.
```bash
npx create-expo-app@latest
```

If you want a different starter (blank JS, blank TS, tabs, etc.), you can run:
By default, you get a TypeScript Expo app with Expo Router configured.

```bash
npx create-expo-app@latest --template
```
If you want a different starter (blank JS, blank TS, tabs, etc.), you can run:

And pick whichever template fits your workflow.
```bash
npx create-expo-app@latest --template
```

And pick whichever template fits your workflow.

Since this repository is already created, install dependencies and start the development server:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Clarify installation instructions for cloned repository.

The Quick Start section includes generic npx create-expo-app instructions, but then states "Since this repository is already created, install dependencies..." This is confusing for users who have already cloned the repo in the previous step. Consider removing lines 114-128 or clearly separating "Creating a new Expo app from scratch" vs. "Using this cloned repository" workflows.

🤖 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 `@README.md` around lines 112 - 130, Update the Quick Start section in README
to separate two clear workflows: "Create a new Expo app from scratch" (show the
npx create-expo-app and --template examples) and a distinct "Using this cloned
repository" subsection that explains to run npm/yarn/pnpm install and start the
dev server; remove or reword the confusing sentence that mixes both flows and
ensure each subsection has its own header and the appropriate commands so users
who cloned the repo are not shown the create-app steps as the next action.

Comment thread README.md
```

Replace the placeholder values with your real credentials. You can get these by signing up at: [**Expo**](https://jsm.dev/nativesub-exp), [**PostHog**](https://jsm.dev/nativesub-posthog) .
Replace the placeholder values with your real credentials. You can get these by signing up at: [**Expo**](https://jsm.dev/nativesub-exp).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix incorrect service reference in environment setup.

The text says "You can get these by signing up at: Expo" but the environment variable is EXPO_PUBLIC_CLERK_PUBLISHABLE_KEY, which is a Clerk credential, not an Expo credential.

📝 Suggested fix
-Replace the placeholder values with your real credentials. You can get these by signing up at: [**Expo**](https://jsm.dev/nativesub-exp).
+Replace the placeholder values with your real credentials. You can get these by signing up at: [**Clerk**](https://jsm.dev/nativesub-clerk).
📝 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.

Suggested change
Replace the placeholder values with your real credentials. You can get these by signing up at: [**Expo**](https://jsm.dev/nativesub-exp).
Replace the placeholder values with your real credentials. You can get these by signing up at: [**Clerk**](https://jsm.dev/nativesub-clerk).
🤖 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 `@README.md` at line 167, The README incorrectly directs users to "Expo" for
credentials while the env var is EXPO_PUBLIC_CLERK_PUBLISHABLE_KEY (a Clerk
credential); update the text to reference Clerk instead of Expo and point the
signup/link to Clerk's signup or documentation (e.g., Clerk signup URL) so the
instruction matches the EXPO_PUBLIC_CLERK_PUBLISHABLE_KEY variable name.

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