-
Notifications
You must be signed in to change notification settings - Fork 19
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: Added analytics logging on non-admin pages access #229
base: develop
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@DedsecKnight i think we should disable analytics while in development? |
Good point! Will make that change once I got everything sorted out for this feature. |
wait then why is this pr not a draft? |
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.
I'm concerned that unconditionally placing the analytics calls in useEffect
hooks will not provide the correct behavior for analytics. As noted by the docs, useEffect
without dependencies runs after every re-render, so it might cause spurious event logs. Instead, we might look into listening to Next.js router events for logging, as detailed in this Next.js + Firebase analytics tutorial. This tutorial show how to use the built-in screen-view tracking feature of Firebase analytics (as opposed to logging custom events for each page).
@DedsecKnight does this require an extra env variable? |
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.
Can you remove all the imports you added when you had analytics in each individual page?
@thetechie7 I am not sure what extra env variable you are referring to in this case. |
No description provided.