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

[Security Solution] JSON diff view PoC (react-diff-view, DMP-only) #172293

Closed
wants to merge 11 commits into from

Conversation

nikitaindik
Copy link
Contributor

@nikitaindik nikitaindik commented Nov 30, 2023

Summary

It's a modified version of this original PR #172124. The goal is to check how getting rid of one of the diffing libs affect the bundle size.

The original PR adds 107.8KB to bundle size.

The original PR uses both "diff" and "diff-match-patch" libraries for computing the diff, depending on which algorithm is used.

In this PR I tried to use only diff-match-patch without using diff.

Looks like it's not possible to completely get rid of the diff library, because we need to use library called unidiff, which lists diff as it dependency.

unidiff is used to convert two strings (old and new) to a single "unified diff"-formatted string. Since unidiff uses diff under the hood, diff still gets bundled.

I tried relying on diff-match-patch methods to produce "unified diff" out of two strings. It didn't work, because the diff format returned by diff-match-patch is slightly different from what unidiff produces. react-diff-view supports only unidiff / diff formatted diff strings.

Another version of this PR that uses only diff and doesn't use diff-match-match: #172299

@nikitaindik nikitaindik self-assigned this Nov 30, 2023
@kibana-ci
Copy link
Collaborator

kibana-ci commented Nov 30, 2023

💔 Build Failed

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 4737 4753 +16

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 12.9MB 13.0MB +107.3KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @nikitaindik

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.

4 participants