feat: Add build flags to allow noop iframe/canvas/shadow dom managers#114
Merged
feat: Add build flags to allow noop iframe/canvas/shadow dom managers#114
Conversation
lforst
reviewed
Oct 17, 2023
Member
Author
|
PR to implement this in replay: getsentry/sentry-javascript#9274 |
Lms24
reviewed
Oct 17, 2023
Member
Lms24
left a comment
There was a problem hiding this comment.
For now, we'll probably keep this enabled by default, but at least we have a path for users to shake this out if they don't need these features
I think this is a good approach. This means it works like our other tree shaking flags and users can opt out in the same way. Easier documentation and more consistency across our packages 👍
billyvg
approved these changes
Oct 17, 2023
AbhiPrasad
approved these changes
Oct 17, 2023
lforst
approved these changes
Oct 18, 2023
mydea
added a commit
to getsentry/sentry-javascript
that referenced
this pull request
Oct 19, 2023
This depends on getsentry/rrweb#114 to be merged first, but allows to configure build time flags to shake out certain rrweb features that may not be used. It also adds a size limit entry that shows the total bundle size with everything that can be shaken out removed, incl. debug stuff. Bundle size is about ~11kb gzipped less in this scenario, which is not bad.
billyvg
pushed a commit
that referenced
this pull request
Oct 20, 2023
…#114) This PR adds 3 new build flags to rrweb: * `__RRWEB_EXCLUDE_CANVAS__` * `__RRWEB_EXCLUDE_SHADOW_DOM__` * `__RRWEB_EXCLUDE_IFRAME__` If you set these to `true` at build time, it will replace the regular `ShadowDomManager` / `CanvasManager` / `IframeManager` with a noop variant of these managers. All of these together shave off about 8 KB gzipped from our replay bundles, if set to `true`. For now, we'll probably keep this enabled by default, but at least we have a path for users to shake this out if they don't need these features. Note: I played with some other approaches, e.g. instead of having the noop class make these e.g. `iframeManager: IframeManager | undefined`, but I think overall the code to guard against using this everywhere ends up being a similar amount of bytes, plus we need to spread this much more through the codebase, making rebasing on upstream master etc. potentially harder. This way, IMHO it should be the easiest way to keep this as contained as possible.
billyvg
pushed a commit
that referenced
this pull request
Apr 26, 2024
…#114) This PR adds 3 new build flags to rrweb: * `__RRWEB_EXCLUDE_CANVAS__` * `__RRWEB_EXCLUDE_SHADOW_DOM__` * `__RRWEB_EXCLUDE_IFRAME__` If you set these to `true` at build time, it will replace the regular `ShadowDomManager` / `CanvasManager` / `IframeManager` with a noop variant of these managers. All of these together shave off about 8 KB gzipped from our replay bundles, if set to `true`. For now, we'll probably keep this enabled by default, but at least we have a path for users to shake this out if they don't need these features. Note: I played with some other approaches, e.g. instead of having the noop class make these e.g. `iframeManager: IframeManager | undefined`, but I think overall the code to guard against using this everywhere ends up being a similar amount of bytes, plus we need to spread this much more through the codebase, making rebasing on upstream master etc. potentially harder. This way, IMHO it should be the easiest way to keep this as contained as possible.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds 3 new build flags to rrweb:
__RRWEB_EXCLUDE_CANVAS____RRWEB_EXCLUDE_SHADOW_DOM____RRWEB_EXCLUDE_IFRAME__If you set these to
trueat build time, it will replace the regularShadowDomManager/CanvasManager/IframeManagerwith a noop variant of these managers.All of these together shave off about 8 KB gzipped from our replay bundles, if set to
true.For now, we'll probably keep this enabled by default, but at least we have a path for users to shake this out if they don't need these features.
Note: I played with some other approaches, e.g. instead of having the noop class make these e.g.
iframeManager: IframeManager | undefined, but I think overall the code to guard against using this everywhere ends up being a similar amount of bytes, plus we need to spread this much more through the codebase, making rebasing on upstream master etc. potentially harder. This way, IMHO it should be the easiest way to keep this as contained as possible.Note 2: In scenarios where you do not set the build flags at all (so neither to true or false), this will actually slightly increase the bundle size 😬 but it's a very tiny increase which is OK I'd say.