-
Notifications
You must be signed in to change notification settings - Fork 120
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: web vitals 3000 fallback #1404
Conversation
@brunomguimaraes is attempting to deploy a commit to the PostHog Team on Vercel. A member of the Team first needs to authorize it. |
@@ -200,7 +200,10 @@ export class WebVitalsAutocapture { | |||
if (!isUndefined(posthogExtensions)) { | |||
;({ onLCP, onCLS, onFCP, onINP } = posthogExtensions.postHogWebVitalsCallbacks) | |||
} | |||
|
|||
else if (assignableWindow.postHogWebVitalsCallbacks) { |
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.
hey @brunomguimaraes this shouldn't be necessary, what's the problem this solves?
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.
Ugh, dang... It's because we load the latest web vitals extension into older clients. What a pain 😭
AFK right now will look at this properly when back
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.
Exactly @pauldambra !
I've probably should've explain that better on the PR
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 should have caught it... have opened #1407 to try and work towards making this all safer
hey @brunomguimaraes this wouldn't fix the issue for old clients since they wouldn't have this new code i've opened #1406 instead, thanks for flagging this though |
#1401