-
Notifications
You must be signed in to change notification settings - Fork 13
chore: Update Next integration strategy #50
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
Conversation
joshsny
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love it - left a couple small comments to address - looking forward getting this improvement shipped!
|
|
||
| When we make improvements to this process, these are available instantly to all users of the wizard, no training delays or other ambiguity. | ||
|
|
||
| ## Testing locally |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💖
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can also use pnpm try if you don't want to rebuild each time :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh get this: doesn't work if we need to detect a version as in instrumentation-client. Gotta actually build and run it from the directory
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless I'm holding it wrong?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry that was very poorly documented, you can do pnpm try --install-dir=e2e-tests/test-applications/nextjs-app-router-test-app for running in an app and that should work without a build :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay that works!
if I drag and drop a folder into the CLI, as in
pnpm try --install-dir='/Users/daniloc/Documents/Dev/wizard/e2e-tests/test-applications/nextjs-app-router-test-app'
I get an error. an issue escaping the quotes that are auto populated maybe?
daniloc@DAir wizard % pnpm try --install-dir='/Users/daniloc/Documents/Dev/wizard/e2e-tests/test-applications/nextjs-app-router-test-app'
> @posthog/wizard@1.2.2 try /Users/daniloc/Documents/Dev/wizard
> ts-node bin.ts --install-dir\=/Users/daniloc/Documents/Dev/wizard/e2e-tests/test-applications/nextjs-app-router-test-app
┌ Welcome to the PostHog setup wizard ✨
│
■ Could not find package.json. Make sure to run the wizard in the root of your app!
│
└ Wizard setup cancelled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm does it work without the quotes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
path resolution bug; will patch
…on-strategy # Conflicts: # src/nextjs/nextjs-wizard.ts
* Use history_change instead of a manual pageview function call * Add docs for instrumentation-client and logic to select * Update next wizard for instrumentation-client file * Update readme with local testing guidance * Move testing content * Fix comment * Prettier * const (okay face) * 😑 * Add instrumentation-client test app * Update output message * Remove stray import * Allow absolute paths as --install-dir values * Update readme * Make it copyable * well that didn’t help
This pull:
integration-client.ts|js, gated upon version (15.3 and up): https://nextjs.org/docs/app/api-reference/file-conventions/instrumentation-clientPostHogPageView()component stuff in favor of the newhistory_changefeature