Skip to content
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: Remove getCanvasManager, export CanvasManager class directly #153

Merged
merged 9 commits into from
Jan 10, 2024

Conversation

billyvg
Copy link
Member

@billyvg billyvg commented Jan 9, 2024

This simplifies the code a bit by exporting the CanvasManager directly. With ReplayCanvas, we can rely on it for complex setup, but keeps it simple for our users.

This simplifies the code a bit by exporting the CanvasManager directly. With [ReplayCanvas](getsentry/sentry-javascript#10112), we can rely on it for complex setup, but keeps it simple for our users.
This simplifies the code a bit by exporting the CanvasManager directly. With [ReplayCanvas](getsentry/sentry-javascript#10112), we can rely on it for complex setup, but keeps it simple for our users.
@billyvg billyvg force-pushed the feat-fix-tree-shaking-canvas-manager branch from 5c0484b to 9021d0d Compare January 9, 2024 21:56
@billyvg billyvg marked this pull request as ready for review January 9, 2024 21:57
@billyvg billyvg requested a review from a team January 9, 2024 21:57
Copy link

github-actions bot commented Jan 9, 2024

size-limit report 📦

Path Size
rrweb - record only (gzipped) 16.67 KB (-0.15% 🔽)
rrweb - record & getCanvasManager only (gzipped) 0 B (removed)
rrweb - record only (min) 56.91 KB (-0.07% 🔽)
rrweb - record with treeshaking flags (gzipped) 15.46 KB (-0.17% 🔽)
rrweb - record & CanvasManager only (gzipped) 19.21 KB (added)

@billyvg billyvg requested a review from mydea January 9, 2024 23:56
@mydea
Copy link
Member

mydea commented Jan 10, 2024

hmm if this does not mess with tree shaking (always hard to say in this code base 😬 ) then looks good to me!

@billyvg
Copy link
Member Author

billyvg commented Jan 10, 2024

Seems like tree shaking still works according to size-limit!

@billyvg billyvg merged commit 2fa46f3 into sentry-v2 Jan 10, 2024
14 checks passed
@billyvg billyvg deleted the feat-fix-tree-shaking-canvas-manager branch January 10, 2024 15:41
billyvg added a commit that referenced this pull request Apr 26, 2024
#153)

This simplifies the code a bit by exporting the CanvasManager directly.
With
[ReplayCanvas](getsentry/sentry-javascript#10112),
we can rely on it for complex setup, but keeps it simple for our users.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants