Conversation
feat: react support
fix: add react option when not detected
* chore: basic publishing workflow * chore: add husky * fix: formatting on docs * fix: skip install test for now * fix: linter issues
* chore: basic publishing workflow * chore: add husky * fix: formatting on docs * fix: skip install test for now * fix: linter issues * chore: update readme
* feat(wip): detect env var prefix * feat: env var detection * chore: formatting * chore: update vite imports, move cra file * fix: formatting * chore: add a tanstack test app * fix: formatting
joshsny
left a comment
There was a problem hiding this comment.
Left some comments, this is going to be a great addition 🙌
|
|
||
| # Naming | ||
|
|
||
| Before creating any new event or property names, consult with the developer for any existing naming convention. Consistency in naming is essential. Similarly, be careful about any changes to existing naming as this may break reporting and distort data for the project. No newline at end of file |
There was a problem hiding this comment.
I think here we can just ask it to be consistent, rather than consulting with the developer as we want things to move fairly fast when adding analytics
There was a problem hiding this comment.
Okay so that's a piece where moving fast could lead the developer right over a cliff. Two essential pieces of guidance to not create a Failed Integration:
- collect all platforms in a single project (so web, iOS, android for example)
- use consistent event names across those platforms
My big concern from talking to the sales crew is how much event naming drift causes problems. But maybe the above is the wrong way to address this. What do you think?
There was a problem hiding this comment.
Sounds good, let's keep it and try it out!
FYI I'll probably be working on a custom version of instrumenting events so will be nice to see how these cursor rules play out as a first step
|
|
||
| # Custom properties | ||
|
|
||
| If a custom property is at any point referenced in two or more files or two or more callsites in the same file, use an enum or const object, as above in feature flags. |
There was a problem hiding this comment.
For properties, I'd suggest we don't enforce the enum constraint as it makes code edits more complex for the LLM, but to encourage adding as many useful properties as possible. Thoughts?
There was a problem hiding this comment.
Okay so I am deeply allergic to using the same string literal more than once, because if you ever have to change its value, you don't have a single source of truth to work from. but maybe that doesn't matter as much for custom properties. If you're setting the same property in multiple places maybe that is itself the problem.
On "useful properties," hmm, agree it would be nice but I wonder how to constrain that given how stupid these tools are when taking initiative lol
There was a problem hiding this comment.
Maybe you can take these rules for a spin in cursor and see whether it pulls out the properties well?
If it does, then the end result would be great. My concern was if this will slow it down and distract it as it's switching between writing to different files a lot 🤷♂️
There was a problem hiding this comment.
So the approach that it took was to create a new file dedicated to these constants, which was perfect. That's narrow enough not to clog the context window, and the context it does consume is very helpful
It would also be handy to then be able to @ the file in the future:
"All the names you need to use or update are in @analytics.ts" or what have you
I wonder if that should be an explicit part of the prompt, making that file.
|
|
||
| # Identification | ||
|
|
||
| How PostHog identifies users and whether events are identified have significant billing consequences for an integration. Consult with the developer before writing any code to implement or alter the approach to this task. |
There was a problem hiding this comment.
This we can probably leave, as by default the installation does not identify from the wizard, so the LLM is unlikely to add it unless there is clearly auth related code, in which case we usually want to encourage identification.
I think it'd be useful to prompt the user around how to identify well (e.g. when the user logs in) and remember to reset when they logout etc.
…to add-rules-step # Conflicts: # package.json # src/nextjs/nextjs-wizard.ts
…the robot will ignore
|
Okay, so here's a simple but working starting place, evolved based on the above discussion. I've debugged it extensively (we need to store the files as .md instead of .mdc otherwise using cursor OURSELVES will cause the frontmatter for driving the UI to be mangled, and the rules don't get applied in the target projects) Took the rules THEMSELVES for a test drive and:
Playing with this, I think it's probably helpful to start with a light touch and then evolve the rules files as we learn more from feedback. That said, any other obvious cues we'd want to include to support other products/features? |
* feat(wip): react setup * fix: add back some changes from merge * feat(wip): pulling out nextjs * feat(wip): pull out nextjs internals * chore: remove posthog-js / posthog-node from package.json * feat(wip): react support * chore: ditch semver on react-version * chore: lint * chore: specify to use '.js' for next config * Naive addition of cursor rules * Only install cursor rules if we're running in a cursor session * chore: fix analytics, add back empty next config in example app * 0.2.11 * fix: add react option when not detected * 0.3.0 * chore: initial action for publishing on a new version (#10) * chore: basic publishing workflow * chore: add husky * fix: formatting on docs * fix: skip install test for now * fix: linter issues * Update readme for react (#11) * chore: basic publishing workflow * chore: add husky * fix: formatting on docs * fix: skip install test for now * fix: linter issues * chore: update readme * chore: add tracking for current integration (#12) * feat: detect env var prefix + imports in react (#13) * feat(wip): detect env var prefix * feat: env var detection * chore: formatting * chore: update vite imports, move cra file * fix: formatting * chore: add a tanstack test app * fix: formatting * 0.4.0 * 0.4.1 * Rework the rules structure * Adopt new rules structure * Naive addition of cursor rules * Rework the rules structure * Extract rules function for any integration to call * Finish dealing with merge * Store source rules as .md so Cursor doesn’t mangle their frontmatter * Lead with the more correct TS approach then degrade to JS, otherwise the robot will ignore * chore: run prettier, add integration tracking, trace step * chore: formatting * chore: add some tests * chore: formatting * chore: rename to editor rules for future editor support * chore: ask user if they want rules, default to true * 0.4.2 * 0.5.0 * fix: update postbuild * fix: clack select in tests --------- Co-authored-by: Joshua Snyder <joshua@posthog.com>
Adding Cursor rules files is pretty straightforward. This could be handy because the agent is liable to take random initiative and break perfectly working things. Hypothesis: A bit of static error correction built into the project will guard against the worst drift.
Even more interesting: the wizard could keep the rules up to date as our docs evolve.
This pull:
CURSOR_TRACE_IDenvironment variable