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

Publish our forked rrweb packages to NPM #15

Closed
10 tasks done
Lms24 opened this issue Jan 13, 2023 · 3 comments
Closed
10 tasks done

Publish our forked rrweb packages to NPM #15

Lms24 opened this issue Jan 13, 2023 · 3 comments
Assignees

Comments

@Lms24
Copy link
Member

Lms24 commented Jan 13, 2023

Problem Statement

Because of bugs in rrweb, its slow development cycle, and the fact that they're working on v2 (which is also problematic on its own, getsentry/sentry-javascript#6655) we have to apply multiple patches to the library, and bundle it into our @sentry/replay package. This has a number of drawbacks:

Solution Brainstorm

In a first step, to make developing patches on rrweb v1 easier, we update our rrweb fork in which we now create and merge PRs which we then use to patch the actual rrweb package.

We now propose to release our rrweb fork (e.g. @sentry-internal/rrweb) to NPM so we can stop patching the official package all together and just use our fork like a regular dependency. The steps to get this done are:

#15 (comment)

#15 (comment)

#15 (comment)

For the most part, we can follow our SDK release checklist to make sure we've covered everything necessary.

Furthermore, we can also take getsentry/sentry-javascript-bundler-plugins#60 as an inspiration, where we recently started releasing to NPM from a new repo.

@Lms24
Copy link
Member Author

Lms24 commented Jan 13, 2023

cc @bruno-garcia @mydea @billyvg

@Lms24
Copy link
Member Author

Lms24 commented Jan 13, 2023

Arguably, this issue rather belongs into https://github.com/getsentry/rrweb but we currently can't create issues there. Happy to move it, once we can open issues (@bruno-garcia do you have the necessary permissions to enable issues?)

@bruno-garcia
Copy link
Member

(@bruno-garcia do you have the necessary permissions to enable issues?)

image

@Lms24 Lms24 transferred this issue from getsentry/sentry-javascript Jan 17, 2023
@Lms24 Lms24 self-assigned this Jan 19, 2023
Lms24 added a commit to getsentry/sentry-javascript that referenced this issue Jan 26, 2023
As outlined in getsentry/rrweb#15, we're now releasing packages of our rrrweb fork:

* replace the original rrweb dependency with our forked one
* remove the patches for the original rrweb dependency which we now don't need anymore
* remove the `patch-package` dev dependency, as we're not patching any package anymore

In terms of building the rrweb package, nothing changes - we're still inlining the rrweb code.
@Lms24 Lms24 closed this as completed Feb 20, 2023
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

No branches or pull requests

2 participants