-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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(replay): Add ReplayCanvas
integration
#10112
Conversation
size-limit report 📦
|
@mydea works if it's in a sep package, but still need to test that canvas recording still works (though the e2e test should confirm this) |
hmm wtf, but well, good :D if that fixes it, amazing! |
@mydea a few questions:
|
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.
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.
@@ -32,6 +32,10 @@ targets: | |||
- name: npm | |||
id: '@sentry-internal/feedback' | |||
includeNames: /^sentry-internal-feedback-\d.*\.tgz$/ | |||
## 1.8 ReplayCanvas package (browser only) |
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.
Do we need to publish this? Or could we just inline this into browser/replay, same as we do with e.g. replay-worker? IMHO if we can avoid to publish it, we should!
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.
@mydea I added this back, it's been a pain trying to test this package without it being published. e.g. in my test app, I'll use yalc
to add @sentry/browser @sentry/replay
, but yarn will complain:
error Couldn't find package "@sentry-internal/replay-canvas@7.93.0" required by "@sentry/browser@file:.yalc/@sentry/browser" on the "npm" registry.
packages/browser-integration-tests/suites/replay/canvas/template.html
Outdated
Show resolved
Hide resolved
3e11d66
to
86ce591
Compare
#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.
b39c5fe
to
d001529
Compare
9dd8cfe
to
b87f421
Compare
This reverts commit f3290c4.
#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.
Adding this integration in addition to
Replay
will set up canvas recording.