-
Notifications
You must be signed in to change notification settings - Fork 160
feat: push replay and person links into external integrations #1980
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
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Size Change: -11 kB (-0.31%) Total Size: 3.52 MB
ℹ️ View Unchanged
|
This will let us extend our code much more easily in the future. We're also saving us some bytes :)
A `Record` requires all keys to exist but we dont need that, let's use a mapped object instead
I've rerun CI several times and this keeps failing, I dont think it's flaky, we might be using something that doesnt work on IE11 |
weirdly it's almost all types or external and i haven't added the externals to the es5 bundle 😡 |
} | ||
|
||
public startIfEnabledOrStop() { | ||
for (const [key, value] of Object.entries(this._instance.config.integrations ?? {})) { |
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.
ie11 giving
JavaScript error details:
TypeError: Object doesn't support property or method 'entries'
at t.startIfEnabledOrStop
but we polyfill it https://github.com/PostHog/posthog-js/pull/1980/files#diff-001e332a80712cf09955389a80c7dd2eaf800699391a5145792eb41337dd18eeR8
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.
Ah! This is Object.entries
not Object.fromEntries
dangnammit
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.
ok, before this polyfill we had
dist/array.full.es5.js | 300 kB | +914 B (+0.31%)
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.
dist/array.full.es5.js | 301 kB | +1.73 kB (+0.58%)
not too bad... assuming it works
ok @rafaeelaudibert @veryayskiy this is i think ready to go in |
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.
Code looks good, just need to merge with master
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 |
will recreate here instead #2039 |
if you've got intercom or crisp chat on your site, you might want to auto-link session replay to user interactions
you can do this manually but it might be nice to make it easy for folk
crisp
intercom