fix(pro-marketing): nav reflects auth state, hide pro banner for pro users#3250
Conversation
…o users
Two related signed-in-experience bugs caught by the user during the
post-purchase flow:
1. /pro Navbar's SIGN IN button never reacted to auth state. The
component was a static const Navbar = () => <nav>...</nav>; with
no Clerk subscription, so signing in left the SIGN IN button in
place even though the user was authenticated.
2. The "Pro is launched — Upgrade to Pro" announcement banner on the
main app showed for ALL visitors including paying Pro subscribers.
Pitching upgrade to a customer who already paid is a small but
real annoyance, and it stays sticky for 7 days via the localStorage
dismiss key — so a returning paying user dismisses it once and
then never sees the (genuinely useful) banner again if they later
downgrade.
## Changes
### pro-test/src/App.tsx — useClerkUser hook + ClerkUserButton
- New useClerkUser() hook subscribes to Clerk via clerk.addListener
and returns { user, isLoaded } so any component can react to auth
changes (sign-in, sign-out, account switch).
- New ClerkUserButton component mounts Clerk's native UserButton
widget (avatar + dropdown with profile/sign-out) into a div via
clerk.mountUserButton — inherits the existing dark-theme appearance
options from services/checkout.ts::ensureClerk.
- Navbar swaps SIGN IN button for ClerkUserButton when user is
signed in. Slot is intentionally empty during isLoaded=false to
avoid a SIGN IN → avatar flicker for returning users.
- Hero hides its redundant SIGN IN CTA when signed in; collapses to
just "Choose Plan" which is the relevant action for returning users.
- Public/pro/ rebuilt to ship the change (per PR #3229's bundle-
freshness rule).
### src/components/ProBanner.ts — premium-aware show + reactive auto-hide
- showProBanner returns early if hasPremiumAccess() — same authoritative
signal used by the frontend's panel-gating layer (unions API key,
tester key, Clerk pro role, AND Convex Dodo entitlement).
- onEntitlementChange listener auto-dismisses the banner if a Convex
snapshot arrives mid-session that flips the user to premium (e.g.
Dodo webhook lands while they're sitting on the dashboard). Does NOT
write the dismiss timestamp, so the banner reappears correctly if
they later downgrade.
## Test plan
### pro-test (sign-in UI)
- [ ] Anonymous user loads /pro → SIGN IN button visible in nav.
- [ ] Click SIGN IN, complete Clerk modal → button replaced with
Clerk's UserButton avatar dropdown.
- [ ] Open dropdown, click Sign Out → reverts to SIGN IN button.
- [ ] Hard reload as signed-in user → SIGN IN button never flashes;
avatar appears once Clerk loads.
### main app (banner gating)
- [ ] Anonymous user loads / → "Pro is launched" banner shows.
- [ ] Click ✕ to dismiss → banner stays dismissed for 7 days
(existing behavior preserved).
- [ ] Pro user (active Convex entitlement) loads / → banner does
NOT appear, regardless of dismiss state.
- [ ] Free user opens /, then completes checkout in another tab and
Convex publishes the entitlement snapshot → banner auto-hides
in the dashboard tab without reload.
- [ ] Pro user whose subscription lapses (validUntil < now) → banner
reappears on next page load, since dismiss timestamp wasn't
written by the entitlement-change auto-hide.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR fixes two signed-in-experience bugs: the
Confidence Score: 4/5Safe to merge with awareness of the banner-flash regression for Convex-only pro users on initial page load One P1 defect: paying users whose premium access is backed solely by Convex Dodo entitlement will see the Upgrade to Pro banner on every page load until the Convex subscription delivers its first snapshot. The onEntitlementChange auto-dismiss mitigates the symptom but does not prevent the initial flash. All other findings are P2. The Navbar/Hero auth changes are correct and well-guarded. src/components/ProBanner.ts — the synchronous hasPremiumAccess() check at line 48 needs to account for the async Convex entitlement initialization window Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[App Phase 1 init] --> B[showProBanner called sync]
B --> C{hasPremiumAccess?}
C -->|API key or Clerk role| D[Banner skipped - correct]
C -->|Convex-only pro user| E[currentState is null]
E --> F[Banner shown - flash]
F --> G[Convex subscription initializes async]
G --> H[First snapshot delivered]
H --> I{onEntitlementChange fires}
I -->|hasPremiumAccess now true| J[Banner auto-dismissed]
I -->|still free user| K[Banner stays visible]
Reviews (1): Last reviewed commit: "fix(pro-marketing): reflect auth state i..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Subscribe to entitlement changes so a user who upgrades mid-session has | ||
| // the banner auto-dismissed on the next snapshot, instead of staring at | ||
| // their old "go pro" prompt while their just-paid plan is already active. | ||
| if (hasPremiumAccess()) return; |
There was a problem hiding this comment.
hasPremiumAccess() is called synchronously here, but isEntitled() (folded into isProUser()) reads currentState which is always null at this point — initEntitlementSubscription is fired with void (non-awaited) at App.ts:868, and showProBanner is called just a few lines later at App.ts:923 during Phase 1. For a paying user whose premium access is backed only by the Convex Dodo entitlement (no Clerk role, no API key), hasPremiumAccess() returns false and the banner is shown on every page load. The onEntitlementChange auto-dismiss eventually corrects this, but only after the full waitForConvexAuth(10_000) + first Convex snapshot roundtrip — potentially several seconds of visible banner.
Consider deferring the first showProBanner call until initEntitlementSubscription settles, or checking entitlement asynchronously before inserting the banner element:
// In App.ts, await entitlement init before showing the banner
await initEntitlementSubscription(userId);
showProBanner(this.state.container);Alternatively, add a brief async gate inside showProBanner itself so the DOM insert only happens after the first Convex snapshot has been processed.
| onEntitlementChange(() => { | ||
| if (bannerEl && hasPremiumAccess()) { | ||
| bannerEl.classList.add('pro-banner-out'); | ||
| setTimeout(() => { | ||
| bannerEl?.remove(); | ||
| bannerEl = null; | ||
| }, 300); | ||
| } | ||
| }); |
There was a problem hiding this comment.
onEntitlementChange() returns an unsubscribe function, but the return value is silently dropped here. The listener persists for the lifetime of the module — acceptable in production, but in dev with HMR, hot-reloading this module will accumulate duplicate listeners in the entitlement service's listeners Set.
| onEntitlementChange(() => { | |
| if (bannerEl && hasPremiumAccess()) { | |
| bannerEl.classList.add('pro-banner-out'); | |
| setTimeout(() => { | |
| bannerEl?.remove(); | |
| bannerEl = null; | |
| }, 300); | |
| } | |
| }); | |
| const _unsubProBanner = onEntitlementChange(() => { | |
| if (bannerEl && hasPremiumAccess()) { | |
| bannerEl.classList.add('pro-banner-out'); | |
| setTimeout(() => { | |
| bannerEl?.remove(); | |
| bannerEl = null; | |
| }, 300); | |
| } | |
| }); |
| return () => { | ||
| unmounted = true; | ||
| ensureClerk().then((clerk) => { | ||
| if (el) clerk.unmountUserButton(el); | ||
| }).catch(() => { /* mount path already failed */ }); | ||
| }; |
There was a problem hiding this comment.
unmountUserButton called even when widget was never mounted
If the ClerkUserButton component unmounts before ensureClerk() resolves (e.g. fast auth-state flip), unmounted is set to true which correctly skips mountUserButton, but the cleanup still resolves ensureClerk() and calls clerk.unmountUserButton(el) on an element that never had the widget attached. The .catch() swallows any error thrown, so this won't surface as an exception — but Clerk may log a warning internally.
A simple let widgetMounted = false; flag set to true after mountUserButton succeeds, then checked in the cleanup, would make the guard explicit and avoid calling unmount on a never-mounted element.
Reviewer caught that the previous iteration only handled the upgrade
direction (premium snapshot → hide banner) but never re-showed the
banner on a downgrade. App.ts calls showProBanner() once at init, so
without a symmetric show path, a session that started premium and
then lost entitlement (cancellation, billing grace expiry, plan
downgrade for the same user) would stay banner-less for the rest of
the SPA session — until a full reload re-ran App.ts init.
Net effect of the bug: the comment claiming "the banner reappears
correctly if they later downgrade or the entitlement lapses" was
false in practice for any in-tab transition.
Two changes:
1. Cache the container on every showProBanner() call, including
the early-return paths. App.ts always calls showProBanner()
once at init regardless of premium state, so this guarantees
the listener has the container reference even when the initial
mount was skipped (premium user, dismissed, in iframe).
2. Make onEntitlementChange handler symmetric:
- premium snapshot + visible → hide (existing behavior)
- non-premium snapshot + not visible + cached container +
not dismissed + not in iframe → re-mount via showProBanner
The non-premium re-mount goes through showProBanner() so it gets the
same gate checks as the initial path (isDismissed, iframe, premium).
We can never surface a banner the user has already explicitly ✕'d
this week.
Edge cases handled:
- User starts premium, no banner shown, downgrades mid-session
→ listener fires, premium false, no bannerEl, container cached,
not dismissed → showProBanner mounts banner ✓
- User starts free, sees banner, upgrades mid-session
→ listener fires, premium true, bannerEl present → fade out ✓
- User starts free, dismisses banner, upgrades, downgrades
→ listener fires on downgrade, premium false, no bannerEl,
container cached, isDismissed=true → showProBanner returns early ✓
- User starts free, banner showing, multiple entitlement snapshots
arrive without state change → premium=false && bannerEl present,
neither branch fires, idempotent no-op ✓
|
You're right — the listener was asymmetric. Fixed in `017a99224` with two changes: 1. Cache the container on every `showProBanner()` callApp.ts always calls `showProBanner()` once at init regardless of premium state — the early-returns inside the function decide whether to actually mount. Caching the container reference ahead of those early-returns means the entitlement listener has it available even when the initial mount was skipped (premium user, dismissed, in iframe). ```diff
2. Symmetric `onEntitlementChange` handler```diff
The non-premium re-mount goes through `showProBanner()` so it inherits the same gate set as the initial path (`isDismissed`, iframe, premium re-check). We can never surface a banner the user has already explicitly ✕'d this week. State transition matrix
Comment in source updated to accurately describe the symmetric behavior — the previous "reappears correctly" claim now matches what the code actually does. |
Greptile P1 round-2: hasPremiumAccess() at line 48 reads isEntitled()
synchronously, but the Convex entitlement subscription is fired
non-awaited at App.ts:868 (`void initEntitlementSubscription()`).
showProBanner() runs at App.ts:923 during init Phase 1, before the
first Convex snapshot arrives.
So a Convex-only paying user (Clerk role 'free' + Dodo entitlement
tier=1) sees this sequence:
t=0 init runs → hasPremiumAccess() === false (isEntitled() reads
currentState===null) → "Upgrade to Pro" banner mounts
t=~1s Convex snapshot arrives → onEntitlementChange fires → my
listener detects premium=true && bannerEl !== null → fade out
That's a 1+ second flash of "you should upgrade!" content for someone
who has already paid. Worst case is closer to ~10s on a cold-start
Convex client, which is much worse — looks like the upgrade pitch is
the actual UI.
Defer the initial mount when (1) the user is signed in (so they
plausibly have a Convex entitlement) AND (2) the entitlement state
hasn't loaded yet (currentState === null). The existing
onEntitlementChange listener will mount it later if the first
snapshot confirms the user is actually free.
Two reasons this is gated on "signed in":
- Anonymous users will never have a Convex entitlement, so
deferring would mean the banner NEVER mounts for them. Bad
regression: anon visitors are the highest-value audience for
the upgrade pitch.
- For signed-in users, the worst case if no entitlement EVER
arrives is the banner stays absent — which is identical to a
paying user's correct state, so it fails-closed safely.
Edge case behavior:
- Anonymous user: no Clerk session → first condition false →
banner mounts immediately ✓
- Signed-in free user with first snapshot pre-loaded somehow:
second condition false → banner mounts immediately ✓
- Signed-in user, snapshot pending: deferred → listener mounts
on first snapshot if user turns out free ✓
- Signed-in user, snapshot pending, user turns out premium: never
mounted ✓ (the desired path)
- Signed-in user, snapshot pending, never arrives (Convex outage):
banner never shows → see above, this fails-closed safely
|
You're right — the synchronous `hasPremiumAccess()` at init reads `isEntitled()` against `currentState===null` for the window before the first Convex snapshot arrives. Convex-only paying users would see "Upgrade to Pro" mount, then watch the listener dismiss it ~1s later (worst case ~10s on a cold-start Convex client). Real defect — fixed in `d774516e6`. FixDefer the initial mount when the user is signed in AND entitlement state hasn't loaded: ```diff
The "signed in" gate is critical — anonymous users would otherwise wait forever (no Convex entitlement coming) and the highest-value audience for the upgrade pitch would never see it. Edge case matrix
The "Convex outage" failure mode is the only behavior change for free users: they don't see the banner if entitlement subscription dies entirely. That's a worthwhile tradeoff vs. flashing "Upgrade to Pro" at every paying user on every page load. CI all green (typecheck, biome, unit, changes detection). Ready for re-review. |
1 participant
Two related signed-in-experience bugs caught during the post-purchase flow.
1. /pro Navbar's SIGN IN button never reacted to auth state
Static `const Navbar = () =>
...` with no Clerk subscription. After a successful sign-in, the SIGN IN button stayed in place because the component never re-rendered against `clerk.user`.Fix:
2. "Pro is launched" banner showed for paying Pro users
`src/components/ProBanner.ts::showProBanner` had no entitlement check. Pitching upgrade to a customer who already paid is a small but real annoyance. Plus the banner is sticky for 7 days via the localStorage dismiss key — a returning paying user dismisses it once and then never sees the (genuinely useful) banner again if they later downgrade.
Fix:
Test plan
pro-test (sign-in UI)
main app (banner gating)
Related