-
Notifications
You must be signed in to change notification settings - Fork 121
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
chore: upgrade rrweb to alpha.16 #1276
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
patches/[email protected]
Outdated
- const stylesheet = Array.from(doc.styleSheets).find((s2) => { | ||
- return s2.href === n2.href; | ||
- }); | ||
+ function findStylesheet(href) { | ||
+ return Array.from(doc.styleSheets).find((s2) => { | ||
+ return s2.href === href; | ||
+ }); | ||
+ } | ||
+ let stylesheet = findStylesheet(n2.href); | ||
+ if (!stylesheet && href.includes('.css')) { | ||
+ const stylesheetPath = href.replace(window.location.href, ''); | ||
+ const potentialStylesheetHref = window.location.origin + '/' + stylesheetPath; | ||
+ stylesheet = findStylesheet(potentialStylesheetHref); | ||
+ } |
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.
Replaces
posthog-js/patches/[email protected]
Lines 234 to 247 in 27b044c
- const stylesheet = Array.from(doc.styleSheets).find((s) => { | |
- return s.href === n.href; | |
+ const href = n.href | |
+ let stylesheet = Array.from(doc.styleSheets).find((s) => { | |
+ return s.href === href; | |
}); | |
+ if (!stylesheet && href.includes('.css')) { | |
+ const rootDomain = window.location.origin | |
+ const stylesheetPath = href.replace(window.location.href, '') | |
+ const potentialStylesheetHref = rootDomain + '/' + stylesheetPath | |
+ stylesheet = Array.from(doc.styleSheets).find((s) => { | |
+ return s.href === potentialStylesheetHref; | |
+ }); | |
+ } |
Size Change: +68.6 kB (+2.43%) Total Size: 2.89 MB
ℹ️ View Unchanged
|
patches/[email protected]
Outdated
- const bitmap = await createImageBitmap(canvas); | ||
+ // createImageBitmap throws if resizing to 0 | ||
+ // Fallback to intrinsic size if canvas has not yet rendered | ||
+ const width = canvas.clientWidth || canvas.width; | ||
+ const height = canvas.clientHeight || canvas.height; | ||
+ const bitmap = yield createImageBitmap(canvas, { | ||
+ resizeWidth: width, | ||
+ resizeHeight: height | ||
+ }); | ||
worker.postMessage( | ||
{ | ||
id, | ||
bitmap, | ||
- width: canvas.width, | ||
- height: canvas.height, | ||
+ width: width, | ||
+ height: height, |
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.
Replaces
posthog-js/patches/[email protected]
Lines 204 to 222 in 27b044c
- const bitmap = yield createImageBitmap(canvas); | |
+ | |
+ // createImageBitmap throws if resizing to 0 | |
+ // Fallback to intrinsic size if canvas has not yet rendered | |
+ const width = canvas.clientWidth || canvas.width; | |
+ const height = canvas.clientHeight || canvas.height; | |
+ | |
+ const bitmap = yield createImageBitmap(canvas, { | |
+ resizeWidth: width, | |
+ resizeHeight: height | |
+ }) | |
+ | |
worker.postMessage({ | |
id, | |
bitmap, | |
- width: canvas.width, | |
- height: canvas.height, | |
+ width: width, | |
+ height: height, |
- const bitmap = await createImageBitmap(canvas); | ||
+ // createImageBitmap throws if resizing to 0 | ||
+ // Fallback to intrinsic size if canvas has not yet rendered | ||
+ const width = canvas.clientWidth || canvas.width; | ||
+ const height = canvas.clientHeight || canvas.height; | ||
+ const bitmap = await createImageBitmap(canvas, { | ||
+ resizeWidth: width, | ||
+ resizeHeight: height | ||
+ }); | ||
worker.postMessage( | ||
{ | ||
id, | ||
bitmap, | ||
- width: canvas.width, | ||
- height: canvas.height, | ||
+ width: width, | ||
+ height: height, |
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.
Replaces
posthog-js/patches/[email protected]
Lines 204 to 222 in 27b044c
- const bitmap = yield createImageBitmap(canvas); | |
+ | |
+ // createImageBitmap throws if resizing to 0 | |
+ // Fallback to intrinsic size if canvas has not yet rendered | |
+ const width = canvas.clientWidth || canvas.width; | |
+ const height = canvas.clientHeight || canvas.height; | |
+ | |
+ const bitmap = yield createImageBitmap(canvas, { | |
+ resizeWidth: width, | |
+ resizeHeight: height | |
+ }) | |
+ | |
worker.postMessage({ | |
id, | |
bitmap, | |
- width: canvas.width, | |
- height: canvas.height, | |
+ width: width, | |
+ height: height, |
- win.document.querySelectorAll("canvas").forEach((canvas) => { | ||
- if (!isBlocked(canvas, blockClass, blockSelector, true)) { | ||
- matchedCanvas.push(canvas); | ||
- } | ||
- }); | ||
+ const searchCanvas = (root) => { | ||
+ root.querySelectorAll("canvas").forEach((canvas) => { | ||
+ if (!isBlocked(canvas, blockClass, blockSelector, true)) { | ||
+ matchedCanvas.push(canvas); | ||
+ } | ||
+ }); | ||
+ root.querySelectorAll("*").forEach((elem) => { | ||
+ if (elem.shadowRoot) { | ||
+ searchCanvas(elem.shadowRoot); | ||
+ } | ||
+ }); | ||
+ }; | ||
+ searchCanvas(win.document); |
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.
Replaces:
posthog-js/patches/[email protected]
Lines 177 to 196 in 27b044c
- win.document.querySelectorAll('canvas').forEach((canvas) => { | |
- if (!isBlocked(canvas, blockClass, blockSelector, true)) { | |
- matchedCanvas.push(canvas); | |
- } | |
- }); | |
+ | |
+ const searchCanvas = (root) => { | |
+ root.querySelectorAll('canvas').forEach((canvas) => { | |
+ if (!isBlocked(canvas, blockClass, blockSelector, true)) { | |
+ matchedCanvas.push(canvas); | |
+ } | |
+ }); | |
+ root.querySelectorAll('*').forEach((elem) => { | |
+ if (elem.shadowRoot) { | |
+ searchCanvas(elem.shadowRoot); | |
+ } | |
+ }); | |
+ }; | |
+ | |
+ searchCanvas(win.document); |
damn... i like it |
that's such a big change it makes you worry it's broken 🤣 🤣 |
@@ -10,7 +10,7 @@ app.get('/segment.html', function (req, res) { | |||
}) | |||
|
|||
app.get('/static/recorder.js', function (req, res) { | |||
let filePath = path.join(__dirname, '/../../node_modules/rrweb/dist/rrweb.min.js') |
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.
Doesn't look like they have a min version of the js
file anymore
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.
yuck!
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.
well, less mean-ly i wonder what our minified file looks like now 🤔
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.
🤔 12kb bigger locally but doesn't look tooooo terrible
src/loader-recorder.ts
Outdated
@@ -1,12 +1,6 @@ | |||
import { version } from 'rrweb/package.json' |
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.
Was throwing
(!) Missing global variable names
https://rollupjs.org/configuration-options/#output-globals
Use "output.globals" to specify browser global variable names corresponding to external modules:
rrweb/package.json (guessing "package_json")
rrweb/package.json (guessing "package_json")
I believe the size gains were over reported... things in fact did not work @pauldambra this might need another 👀 because I had do do some weird build fangling to get things working again |
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.
@pauldambra after many rounds of the Yalc dance I think I've managed to fix this (tldr patches are haard, one small mistake and the whole things blows up). Might be worth running it again so that you're happy with things. Could ship today if everything looks good :) |
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.
works locally, that's definitive, ship it
Changes
rrweb moved to vite bundling so the patch file will look different this time around
Previous patch file: https:/PostHog/posthog-js/blob/v1.141.4/patches/rrweb%402.0.0-alpha.13.patch
I skipped the patch replacing the inline worker with the base64 worker
posthog-js/patches/[email protected]
Lines 5 to 164 in 27b044c
It looks like the migration to Vite means that the base64 worker is back. From the built
rrweb.js
file in the /node_modules you can see:This doesn't make total sense to me because the changes made in rrweb-io/rrweb#1309 have not been undone but I'm happy to proceed here and see if it's needed
All other patches have been replaced with comments inline