[FE fix] Post signup survey issues with PostHog#4467
Merged
Conversation
…nal targeting flag The "Signup 2" survey is type=api with schedule=once, which makes PostHog auto-generate an internal_targeting_flag_key (invisible in the dashboard). posthog.getActiveMatchingSurveys runs that flag through the eligibility filter, and for brand-new identified users it evaluates to false before the flag context settles — so the survey is silently filtered out and useSurvey rejects with survey-unavailable. Switch the fetcher to posthog.getSurveys, which returns the raw survey list without the eligibility filter. We filter client-side by name + start/end date. The "show once" decision is owned by our own backend on form submit anyway, so bypassing the SDK-side gate is correct here. Also widen the auto-redirect list in PostSignupForm so transient errors (posthog-unavailable from the load timeout, survey-fetch-error from the new fetch timeout) fall through to /get-started instead of leaving the user on a blank page. We lose survey data on those paths, not signups.
…consumer
Restructures /post-signup to remove the race-condition workarounds that had
accumulated in PostSignupForm. Replaces the single ~480-line component (which
fetched its own posthog, profile, orgs, survey and defended against each being
absent) with a clear separation:
PostSignupRoute (new)
- Owns every async dependency via usePostSignupReadiness.
- Renders one of: <Skeleton>, <Form>, <Fallback>, or fires a router.replace.
- The only place that decides "are we ready to show the form?".
usePostSignupReadiness (new)
- Composes useSurvey + useProfileData + useOrgData into a discriminated
state: loading | ready | skip | fallback.
- "skip" reasons (no API key, survey deleted) → redirect to /get-started.
- "fallback" reasons (transient PostHog or fetch errors) → show a small
"we couldn't load the welcome survey, continue" UI instead of bouncing
the user silently.
PostSignupForm
- Now a pure consumer. Receives survey, user, orgs, posthog as props that
are guaranteed populated. No internal data fetching, no spinner, no
auto-redirect effect, no debug logs.
PostSignupHeader, PostSignupSkeleton, PostSignupFallback (new)
- Small presentational components extracted from the original file so the
loading and fallback states keep the brand header visible.
pages/post-signup/index.tsx
- Slimmed to a 5-line route handler. Debug console.logs removed.
The timeouts inside useSurvey are unchanged — they remain the single source
of truth for "PostHog SDK didn't load" / "survey fetch hung", and the
readiness hook just routes their resulting error codes into the right
rendering branch. Components no longer carry their own timers.
…mpty-form flash Previously, clicking Submit called form.resetFields() before router.push() and the user saw a blank form for the ~200-500ms it took Next.js to navigate to /get-started. Confusing for new signups — looked like the form was wiped out or had silently errored. Three small changes: - Drop the resetFields() call. The page is unmounting anyway, so resetting fields has no effect except triggering that flash. - Add an isSubmitting state that flips on at the start of handleSubmitFormData and persists through navigation (the component unmounts before any reset could matter). - Wrap the form in a Spin with "Setting up your workspace…" and put the submit button in its loading state while isSubmitting is true. The form stays visible but dimmed, so the user gets clear feedback that submission is in flight.
…ness watchdog
Two regressions observed in local testing:
1. New signups landed directly on /get-started without seeing the survey.
Root cause: useSurvey was firing as soon as posthog.__loaded flipped true,
but posthog.surveys (the SDK extension that fetches /api/surveys/) hadn't
finished initializing yet. The top-level posthog.getSurveys at that point
synchronously calls back with [] + {isLoaded:false, error: SURVEYS_NOT_AVAILABLE},
which my code read as "survey not found" and routed to skip → redirect.
Fix: gate the getSurveys call behind posthog.onSurveysLoaded, so by the
time we query for the survey the extension has settled and /api/surveys/
has returned (or errored loudly). This is the part of JP's hotfix that
was correct — the only thing wrong with his fix was using
getActiveMatchingSurveys (eligibility-filtered) rather than getSurveys.
We now have both: wait for surveys-loaded, then read the raw list.
2. Some signups got stuck on the loading skeleton indefinitely. The
readiness hook stayed in `loading` whenever any of (surveyLoading,
profileLoading, posthog, user, survey) was falsy, so a stalled profile
query or a null user-after-profile-loaded kept the spinner alive forever.
Fix: add a 10-second watchdog. If the gate is still in `loading` after
that, it transitions to `fallback` (reason: watchdog-timeout) and
PostSignupFallback shows the existing "we couldn't load the welcome
questionnaire, continue" UI. The 10s budget sits above useSurvey's own
6s timeouts so its specific errors still take precedence, and it's a
single backstop for anything else (profile, orgs, future dependencies).
Bonus cleanup in useSurvey: the SDK's onSurveysLoaded fires its callback
synchronously when surveys are already loaded, which used to race the
unsubscribe assignment. Guard against that — release the subscription
immediately if the promise already settled inside the call.
…prefetch /get-started The Spin overlay approach was unreliable: antd renders the spinner inside the wrapped Form, and the form briefly unmounts mid-route during Next.js's chunk fetch — so on Submit the user sometimes saw a blank page instead of the loading affordance. Two changes: 1. Replace the Spin-over-Form pattern with a full-page PostSignupSubmitting view that takes over the moment isSubmitting flips true. No overlay positioning, no half-disabled form — just "Setting up your workspace" with a spinner. The form is going to unmount anyway; rendering an unambiguous transition state in its place is clearer than trying to keep the form visible while it's being dismantled. 2. Prefetch /get-started from PostSignupRoute on mount. In dev mode Next.js compiles routes lazily and the post-submit router.push can take 1-3s while the chunk is built; in prod the prefetch is cheap and ensures the route swap is effectively instant. Pages-router prefetch is idempotent so calling it on every mount is safe.
…g navigation The transition view existed but never reached the screen. React 18's automatic batching put setIsSubmitting(true) in the same task as posthog.capture and router.push, and the prefetch we added to /get-started made the route swap fast enough that the new render committed *after* the form unmounted. The user saw the form being torn down — looking empty — right up until /get-started replaced it. Use flushSync to commit the submitting render synchronously, then await one requestAnimationFrame so the browser paints it before we begin the route change. This gives the "Setting up your workspace" view a guaranteed visible moment regardless of how fast the prefetched route swaps in.
Temporary instrumentation to figure out why the PostSignupSubmitting view
isn't reaching the screen on submit. Logs at every step:
[post-signup][diag] PostSignupRoute render (status: ...)
[post-signup][diag] PostSignupRoute MOUNTED / UNMOUNTED
[post-signup][diag] PostSignupForm render (isSubmitting: ...)
[post-signup][diag] PostSignupSubmitting render
[post-signup][diag] handleSubmitFormData start
[post-signup][diag] flushSync(setIsSubmitting=true) done
[post-signup][diag] rAF yielded
[post-signup][diag] posthog.capture('survey sent') called
[post-signup][diag] calling router.push('/get-started')
[post-signup][diag] routeChangeStart / routeChangeComplete
Will be removed once we have the trace.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughExtracted async gating to usePostSignupReadiness; added PostSignupRoute and stateless UI components (Header, Skeleton, Submitting, Fallback); converted PostSignupForm to a prop-driven renderer; hardened survey fetching with explicit timeouts and cleanup; simplified the post-signup page. ChangesPost-signup flow refactoring with readiness gating
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
ardaerzin
changed the title
dosubot
Bot
added
size:XL
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
web/ee/src/components/PostSignupForm/PostSignupSubmitting.tsx (1)
20-22: ⚡ Quick winGate/remove render diagnostics before merge.
This logs on every render and will add production console noise; please guard it behind a dev-only check or remove it.
web/ee/src/components/PostSignupForm/PostSignupRoute.tsx (1)
36-61: ⚡ Quick winTrim or dev-gate route diagnostics.
These logs fire on render, mount, and every route event; consider limiting them to development to avoid noisy production telemetry.
web/ee/src/components/PostSignupForm/PostSignupForm.tsx (1)
105-109: ⚡ Quick winGuard temporary diagnostics behind a dev flag.
These timestamped logs are useful during debugging, but they are too verbose for normal runtime and should be development-only.
Also applies to: 148-168, 233-241
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e9a29a4e-ed59-4ea5-b642-51ef14b8c8e5
📒 Files selected for processing (9)
web/ee/src/components/PostSignupForm/PostSignupFallback.tsxweb/ee/src/components/PostSignupForm/PostSignupForm.tsxweb/ee/src/components/PostSignupForm/PostSignupHeader.tsxweb/ee/src/components/PostSignupForm/PostSignupRoute.tsxweb/ee/src/components/PostSignupForm/PostSignupSkeleton.tsxweb/ee/src/components/PostSignupForm/PostSignupSubmitting.tsxweb/ee/src/components/PostSignupForm/hooks/usePostSignupReadiness.tsweb/ee/src/pages/post-signup/index.tsxweb/oss/src/lib/helpers/analytics/hooks/useSurvey.ts
Three correctness fixes raised in review:
1. useSurvey.isSurveyRunning was treating any survey with a future end_date as
not-running. Now compares both start_date and end_date against "now",
treating end_date === null as "no scheduled end".
2. Survey lookup used name.includes(surveyName), which would also match
"Signup 20" if a future survey is added with that name. Switched to
strict equality.
3. router.push('/get-started') in the submit handler is now awaited; on
rejection we reset isSubmitting so the user can retry instead of being
stuck on the "Setting up your workspace" view forever.
Also removes the temporary [post-signup][diag] console.log statements that
landed in dee7651 — they served their purpose tracking down the dev-server
location issue and shouldn't ship to prod.
Contributor
Railway Preview Environment
Updated at 2026-05-28T10:15:18.012Z |
junaway
changed the title
…st-signup-survey-issues
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
tba
Testing
Verified locally
Added or updated tests
QA follow-up
Checklist
Contributor Resources