Refactor PostHog script to use environment variables

[aspectrr]

Description

Updated PostHog initialization to use environment variables for configuration.

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Code style update (formatting, renaming)
• Code refactor (no functional changes)
• Configuration change
• Test update

Checklist

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works

Release Notes

Labels

[claude]

PR Review: Refactor PostHog script to use environment variables

Summary

This PR updates the PostHog initialization to properly use Astro's define:vars directive for passing environment variables to inline scripts. This is a good fix that addresses a common issue with Astro's build-time vs runtime variable handling.

---

✅ Positive Changes

1. Correct Astro Pattern: Using define:vars is the proper way to pass server-side environment variables to inline scripts in Astro. The previous approach using import.meta.env directly in inline scripts wouldn't work correctly.

2. Added Configuration Option: The addition of person_profiles: 'always' is a sensible default for PostHog configuration.

3. Code Formatting: The reformatting improves readability slightly.

---

⚠️ Issues & Concerns

1. Missing Error Handling / Validation (Medium Priority)

The script doesn't validate that the environment variables are defined before initializing PostHog. If any of these variables are undefined, PostHog will fail silently or behave unexpectedly.

Recommendation:

<script
  is:inline
  define:vars={{
    POSTHOG_KEY: import.meta.env.PUBLIC_POSTHOG_API_KEY,
    POSTHOG_HOST: import.meta.env.PUBLIC_POSTHOG_HOST,
    POSTHOG_DEFAULTS: import.meta.env.PUBLIC_POSTHOG_DEFAULTS,
  }}
>
  // Only initialize if we have the required config
  if (POSTHOG_KEY && POSTHOG_HOST) {
    // ... existing PostHog initialization code ...
    posthog.init(POSTHOG_KEY, {
      api_host: POSTHOG_HOST,
      defaults: POSTHOG_DEFAULTS,
      person_profiles: 'always',
    });
  }
</script>

2. PUBLIC_POSTHOG_DEFAULTS Usage Unclear (Low Priority)

The POSTHOG_DEFAULTS variable is passed but its expected format isn't documented. Is this supposed to be a JSON object? A string? This could cause runtime errors if not properly configured.

Recommendation: Add a comment explaining the expected format, or better yet, parse it if it's JSON:

const defaults = POSTHOG_DEFAULTS ? JSON.parse(POSTHOG_DEFAULTS) : {};
posthog.init(POSTHOG_KEY, {
  api_host: POSTHOG_HOST,
  ...defaults,
  person_profiles: 'always',
});

3. No Documentation Update (Low Priority)

The .env.example file exists but there's no documentation about:

• What values to use for these environment variables
• Whether they're required or optional
• What format PUBLIC_POSTHOG_DEFAULTS should be in

Consider adding comments to .env.example:

# PostHog Analytics Configuration
# Required: Get from PostHog project settings
PUBLIC_POSTHOG_API_KEY=phc_your_key_here
PUBLIC_POSTHOG_HOST=https://app.posthog.com
# Optional: JSON string of additional defaults, e.g., '{"capture_pageview":true}'
PUBLIC_POSTHOG_DEFAULTS=

4. Testing (Medium Priority)

The PR checklist shows that tests weren't added. While this is analytics code that's hard to test end-to-end, consider at least:

• Manual testing that PostHog initializes correctly in both dev and production builds
• Verifying behavior when environment variables are missing
• Checking the browser console for errors

---

🔒 Security Considerations

✅ No issues identified: The use of PUBLIC_ prefixed environment variables is correct for Astro - these are meant to be exposed to the client-side code.

---

🎯 Performance Considerations

✅ No issues identified: The changes don't affect performance. The script remains async and uses the same PostHog loading mechanism.

---

📝 Recommendations

Before merging:

1. Add validation to check if required environment variables exist before initialization
2. Clarify the PUBLIC_POSTHOG_DEFAULTS format and usage
3. Test with missing environment variables to ensure graceful degradation

Nice to have:
4. Update .env.example with comments explaining each variable
5. Update the checklist items that apply (self-review, testing)

---

✨ Overall Assessment

This is a good fix that addresses a real issue with environment variable handling in Astro. With the addition of basic validation and documentation, this would be ready to merge. The core change is solid and follows Astro best practices.

Suggested action: Request changes for the validation issue, then approve.

[Copilot]

Pull request overview

Updates the landing page’s PostHog initialization to inject configuration via Astro define:vars (instead of referencing import.meta.env directly in a raw inline script), and adjusts init options.

Changes:

• Refactors posthog.astro to pass env values into an inline script using define:vars.
• Updates posthog.init(...) options, including adding person_profiles: "always".

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

posthog.init runs unconditionally. If PUBLIC_POSTHOG_API_KEY / PUBLIC_POSTHOG_HOST are unset (the .env.example leaves them blank), this will generate a broken script URL like undefined/static/array.js and may throw at runtime. Consider guarding initialization (and the snippet loader) behind a check that both values are non-empty, or provide safe defaults / a no-op in non-production builds.

Suggested change

	posthog.init(POSTHOG_KEY, {
	api_host: POSTHOG_HOST,
	defaults: POSTHOG_DEFAULTS,
	person_profiles: "always",
	});
	if (POSTHOG_KEY && POSTHOG_HOST) {
	posthog.init(POSTHOG_KEY, {
	api_host: POSTHOG_HOST,
	defaults: POSTHOG_DEFAULTS,
	person_profiles: "always",
	});
	} else {
	console.warn(
	"PostHog not initialized: PUBLIC_POSTHOG_API_KEY or PUBLIC_POSTHOG_HOST is missing.",
	);
	}

[Copilot]

POSTHOG_DEFAULTS is typed as a string (see src/env.d.ts), but it’s passed directly as defaults in the PostHog init options. If this env var is intended to represent structured defaults (e.g., JSON), it should be parsed/validated before being passed; otherwise, consider removing the defaults option to avoid silently misconfiguring PostHog.

Suggested change

	posthog.init(POSTHOG_KEY, {
	api_host: POSTHOG_HOST,
	defaults: POSTHOG_DEFAULTS,
	person_profiles: "always",
	});
	let parsedDefaults = undefined;
	if (typeof POSTHOG_DEFAULTS === "string" && POSTHOG_DEFAULTS.trim() !== "") {
	try {
	const maybeDefaults = JSON.parse(POSTHOG_DEFAULTS);
	if (maybeDefaults && typeof maybeDefaults === "object") {
	parsedDefaults = maybeDefaults;
	}
	} catch (err) {
	// If parsing fails, ignore POSTHOG_DEFAULTS to avoid misconfiguration
	parsedDefaults = undefined;
	}
	}
	const posthogInitOptions = {
	api_host: POSTHOG_HOST,
	person_profiles: "always",
	};
	if (parsedDefaults !== undefined) {
	posthogInitOptions.defaults = parsedDefaults;
	}
	posthog.init(POSTHOG_KEY, posthogInitOptions);

[Copilot]

Setting person_profiles: "always" changes data collection behavior compared to the previous config and may have privacy/compliance implications (e.g., collecting person properties even when not identifying). If this isn’t explicitly desired for all environments, consider gating it behind an environment variable or removing it.