Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: load flagged component after 5 seconds if no flag info #9234

Merged
merged 5 commits into from
Aug 23, 2024

Conversation

raquelmsmith
Copy link
Member

@raquelmsmith raquelmsmith commented Aug 22, 2024

Changes

If feature flags don't resolve, due to them being ad blocked or some other issue, then our RenderInClient component will just show the placeholder forever. This is okay in some cases where the placeholder is actual sane info, but in places like on pricing page experiments where we show a skeleton as a placeholder, this is not great.

This changes it so that, after 5 seconds of rendering the RenderInClient component if we still have no flag info, we just render the render() component. Those components should always use some sort of flag check like posthog?.isFeatureEnabled('direct-to-eu-cloud') ? 'eu' : 'app', so if we don't have flags then we just end up with the default.

Question - isFeatureEnabled is in the snippet. But if posthog isn't loaded yet, will this throw an error? (I loaded the page while blocking the requests and it seemed to work okay so I think it's fine)

Copy link

vercel bot commented Aug 22, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
posthog ✅ Ready (Inspect) Visit Preview Aug 22, 2024 8:41pm

Copy link
Contributor

@benjackwhite benjackwhite left a comment

Choose a reason for hiding this comment

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

I think this is the most sensible choice

@raquelmsmith raquelmsmith merged commit 33176db into master Aug 23, 2024
3 checks passed
@raquelmsmith raquelmsmith deleted the fix/flags-render-default branch August 23, 2024 16:41
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.

2 participants