Skip to content

Conversation

@aspicer
Copy link

@aspicer aspicer commented Oct 17, 2025

Problem

Changes

Release info Sub-libraries affected

Libraries affected

  • All of them
  • posthog-js (web)
  • posthog-js-lite (web lite)
  • posthog-node
  • posthog-react-native
  • @posthog/react
  • @posthog/ai
  • @posthog/nextjs-config

Checklist

  • Tests for new code
  • Accounted for the impact of any changes across different platforms
  • Accounted for backwards compatibility of any changes (no breaking changes!)
  • Took care not to unnecessarily increase the bundle size

If releasing new changes

  • Ran pnpm changeset to generate a changeset file
  • Added the "release" label to the PR to indicate we're publishing new versions for the affected packages

@vercel
Copy link

vercel bot commented Oct 17, 2025

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

Project Deployment Preview Updated (UTC)
posthog-js Ready Ready Preview Oct 17, 2025 5:48pm

💡 Enable Vercel Agent with $100 free credit for automated AI reviews

@aspicer aspicer marked this pull request as ready for review October 17, 2025 22:18
@aspicer aspicer requested a review from a team as a code owner October 17, 2025 22:18
@aspicer aspicer requested a review from a team October 17, 2025 22:18
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

7 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

export function createTypedEventCapture<Client extends { capture: (event: string, properties?: any) => any }>(
client: Client
): TypedEventCapture<Client> {
return new Proxy({} as TypedEventCapture<Client>, {
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Proxy is not supported in IE 11, which is listed in the browserslist. This will cause runtime errors in IE 11 environments when the typed property is accessed.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/core/src/typed-events.ts
Line: 44:44

Comment:
**logic:** `Proxy` is not supported in IE 11, which is listed in the browserslist. This will cause runtime errors in IE 11 environments when the `typed` property is accessed.

How can I resolve this? If you propose a fix, please make it concise.

client: Client
): TypedEventCapture<Client> {
return new Proxy({} as TypedEventCapture<Client>, {
get: (_target, eventName: string) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: The eventName parameter should be typed as string | symbol since Proxy get traps can receive Symbols (e.g., for well-known symbols like Symbol.toStringTag). Consider adding a type guard to filter out non-string property keys.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/core/src/typed-events.ts
Line: 45:45

Comment:
**logic:** The `eventName` parameter should be typed as `string | symbol` since Proxy get traps can receive Symbols (e.g., for well-known symbols like `Symbol.toStringTag`). Consider adding a type guard to filter out non-string property keys.

How can I resolve this? If you propose a fix, please make it concise.

@robbie-c
Copy link
Member

  • We spoke about how you could add types to the builtin capture() method, too, so that existing installations of posthog can be type-checked without needing any code changes
  • It'd be great if you could add an example event schema to packages/browser/playground/nextjs in this PR, so that I can play around with it a bit

@posthog-bot
Copy link
Collaborator

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants