Skip to content

Remove deprecated /register route and fix referral code persistence - #1178

Merged
LiorAgnin merged 1 commit into
fix/analytics-attributionfrom
fix/remove-register-route-referral-persistence
Jan 7, 2026
Merged

Remove deprecated /register route and fix referral code persistence#1178
LiorAgnin merged 1 commit into
fix/analytics-attributionfrom
fix/remove-register-route-referral-persistence

Conversation

@LiorAgnin

Copy link
Copy Markdown
Contributor
  • Remove /register route from app layout and path constants
  • Fix referral code preservation during signup flow reset
  • Add hydration tracking to referral store to prevent race conditions
  • Improve attribution initialization to wait for both stores
  • Add referral code sync verification with Sentry error tracking
  • Implement multi-source fallback for referral code (URL > referral > attribution)

- Remove /register route from app layout and path constants
- Fix referral code preservation during signup flow reset
- Add hydration tracking to referral store to prevent race conditions
- Improve attribution initialization to wait for both stores
- Add referral code sync verification with Sentry error tracking
- Implement multi-source fallback for referral code (URL > referral > attribution)
@vercel

vercel Bot commented Jan 7, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Review Updated (UTC)
solid-app Ignored Ignored Jan 7, 2026 2:50pm
solid-app-staging Ignored Ignored Jan 7, 2026 2:50pm

@LiorAgnin
LiorAgnin merged commit bfa2b67 into fix/analytics-attribution Jan 7, 2026
4 checks passed
@LiorAgnin
LiorAgnin deleted the fix/remove-register-route-referral-persistence branch January 7, 2026 14:50
@claude

claude Bot commented Jan 7, 2026

Copy link
Copy Markdown

Pull Request Review: Remove deprecated /register route and fix referral code persistence

Overview

This PR addresses referral code persistence issues during the signup flow and removes deprecated routes. The changes improve reliability of attribution tracking through better hydration handling and multi-source fallback logic.


✅ Strengths

  1. Good Problem Identification: The PR correctly identifies and fixes race conditions between store hydration and referral code detection
  2. Defensive Fallback Logic: Multi-source fallback (URL > referral store > attribution store) in app/signup/email.tsx:130-134 improves robustness
  3. Verification & Monitoring: Adding Sentry tracking for referral code sync mismatches in hooks/useAttributionInitialization.ts:74-81 provides good observability
  4. Code Cleanup: Removing deprecated /register route reduces maintenance burden

🔴 Critical Issues

1. Race Condition Still Possible in Email Signup (app/signup/email.tsx:80-96)

Severity: High

The reset effect preserves referral code, but there's a timing issue:

// Current code
const existingReferral = getReferralCodeForSignup();
useSignupFlowStore.getState().reset();
if (existingReferral) {
  setReferralCode(existingReferral);
}

Problem: getReferralCodeForSignup() reads from stores that may not be hydrated yet, even though we check _hasHydrated for the signup flow store. The attribution store has its own hydration state.

Fix: Add attribution store hydration check:

const attributionHasHydrated = useAttributionStore(state => state._hasHydrated);

useEffect(() => {
  if (!_hasHydrated || !attributionHasHydrated) return;
  // ... rest of logic
}, [_hasHydrated, attributionHasHydrated, setReferralCode]);

2. Removed Referral Detection May Break User Flows (app/signup/email.tsx:70-81)

Severity: High

The PR removes the detectAndSaveReferralCode() call from the email signup page. This logic was moved to useAttributionInitialization, but:

Problem: The attribution hook only runs at the root _layout.tsx level. If a user:

  • Lands directly on /signup/email?ref=ABC123 (deep link or bookmark)
  • Or if the root layout hasn't mounted yet when they reach this page

The referral code may not be captured.

Recommendation: Either:

  1. Keep the detectAndSaveReferralCode() call as a safety net, OR
  2. Add explicit documentation/testing that referral codes are ONLY captured at root level

3. Incomplete Import Cleanup (app/signup/email.tsx:23)

Severity: Low

The import was changed from:

import { detectAndSaveReferralCode, getReferralCodeForSignup } from '@/lib/utils/referral';

to:

import { getReferralCodeForSignup } from '@/lib/utils/referral';
import { useAttributionStore } from '@/store/useAttributionStore';

But useAttributionStore is only used via getState() in one place (line 133). This could be simplified to:

useAttributionStore.getState().attributionData.referral_code

without importing the hook itself, keeping imports minimal.


⚠️ Potential Issues

4. Console Warnings in Production Code

Severity: Medium

Multiple console.warn() calls throughout:

  • app/signup/email.tsx:95: "✅ Referral code preserved after reset"
  • app/signup/email.tsx:138: "✅ Referral code saved to signup flow"
  • hooks/useAttributionInitialization.ts:69: "✅ Referral code captured"

Problem: These will appear in production builds, potentially exposing internal implementation details and creating noise in user consoles.

Recommendation:

  • Use a debug logging utility that's disabled in production
  • Or convert to Sentry breadcrumbs for production debugging

5. Missing Error Handling for State Access

Severity: Medium

In app/signup/email.tsx:133, direct state access:

useAttributionStore.getState().attributionData.referral_code

Problem: If attributionData is undefined or the store isn't initialized, this could throw.

Fix: Add optional chaining:

useAttributionStore.getState().attributionData?.referral_code || ''

6. Zustand Store Reset Preserves Only One Field (store/useSignupFlowStore.ts:114-120)

Severity: Low

The reset function now preserves referralCode:

reset: () =>
  set(state => ({
    ...initialState,
    _hasHydrated: true,
    referralCode: state.referralCode || initialState.referralCode,
  })),

Concern: If other attribution-related fields get added to the signup flow store in the future, they won't be preserved. The comment says "preserve referralCode captured at root level" but doesn't prevent future bugs.

Recommendation: Consider a more explicit approach:

reset: () => {
  const preservedFields = {
    referralCode: state.referralCode || initialState.referralCode,
  };
  return set({ ...initialState, _hasHydrated: true, ...preservedFields });
}

🧪 Test Coverage

Missing Tests: This PR has no test coverage for:

  • Referral code preservation during signup flow reset
  • Multi-source fallback logic
  • Hydration race condition handling
  • Sentry error reporting for sync mismatches

Recommendation: Add integration tests for:

describe('Referral Code Persistence', () => {
  it('preserves referral code when resetting signup flow')
  it('falls back to attribution store when referral store is empty')
  it('syncs referral code between stores correctly')
  it('reports Sentry error when stores are out of sync')
  it('waits for both stores to hydrate before capturing attribution')
})

🔒 Security Considerations

No major security issues identified, but consider:

  1. Input Validation: Referral codes from URLs should be validated/sanitized before storage (currently handled by the attribution store's sanitizeAttribution, which is good)
  2. PII Protection: The existing PII detection in sanitizeAttribution is solid

🚀 Performance Considerations

Positive:

  • Removing the deprecated route reduces bundle size slightly
  • Hydration checks prevent unnecessary re-renders

Concern:

  • Multiple getState() calls in the signup flow could be optimized by storing the result in a variable

📋 Code Quality

Good:

  • Clear comments explaining the "why" behind complex logic
  • Consistent error handling with Sentry integration
  • Good use of TypeScript

Improvement Opportunities:

  • Remove emoji from production console logs
  • Consolidate duplicate console.warn messages
  • Add JSDoc comments to new/modified functions

Summary & Recommendations

Overall Assessment: This PR makes meaningful improvements to referral tracking reliability, but has a critical issue where removing detectAndSaveReferralCode() from the email page may break direct navigation scenarios.

Before Merging:

  1. Required: Fix the race condition in app/signup/email.tsx by checking attribution store hydration
  2. Required: Verify that referral codes work when users land directly on /signup/email?ref=CODE
  3. ⚠️ Recommended: Add test coverage for referral persistence scenarios
  4. ⚠️ Recommended: Replace console.warn with production-safe logging
  5. ⚠️ Recommended: Add optional chaining for safer store access

Estimated Risk: Medium-High without addressing issues #1 and #2

Great work on improving the attribution system's robustness! The hydration tracking and multi-source fallback are solid architectural improvements. 🎯

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