Replace lodash template with static react renderer for discover's row formatter

[legrego]

Summary

Replaces the rendering logic for Discover's row_formatter. Previous implementation uses lodash.template, which requires unsafe code execution (unsafe-eval in CSP parlance). New implementation uses ReactDOM to render static markup.

This is a prerequisite for hardening our content security policy.

Related: #94626

[kertal]

@elasticmachine merge upstream ACK will have a look today

[kertal]

Code LGTM, 👍 . dangerouslySetInnerHTML is safe here due to the use of field formatters. There is one regression here, performance. so the new implementation needs about double of the time to execute.

Old: 2.72 ms in this case

New: 5.15 ms in this case

This sums up when there are multiple rows rendered. However I don't think this is a blocker, since we recently switched Discover to the use of #89264 , so it's just used by the legacy table. Do you agree dear @timroes ?

[timroes]

@kertal I would consider this to be fine (also security outwins performance anyway). I am also not sure if in this very fast are we can really rely on the "ratio" between those numbers or if there might be too much variation in it. If we can quickly look into the react performance and figure there's an easy win (not sure how often that's rendered, maybe React.memo will help us here) we can still make it, but I am also fine merging it as it is.

[kertal]

@kertal I would consider this to be fine (also security outwins performance anyway). I am also not sure if in this very fast are we can really rely on the "ratio" between those numbers or if there might be too much variation in it. If we can quickly look into the react performance and figure there's an easy win (not sure how often that's rendered, maybe React.memo will help us here) we can still make it, but I am also fine merging it as it is.

about the variation, tested in with multiple docs also, generally, performance regression is about the same

[legrego]

Sounds like the performance impact is tolerable then - thanks for verifying! Would be great to get an official approval whenever y'all get the chance - this isn't a huge priority, I just don't want to forget about it

[legrego]

@elasticmachine merge upstream

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: f371ba1
• Storybooks Preview
• Documentation Changes

Metrics [docs]

Async chunks

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

id	before	after	diff
discover	410.9KB	411.7KB	+747.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
discover	74.6KB	74.7KB	+74.0B

History

• 💚 Build #122227 succeeded 7bb5530
• 💚 Build #122034 succeeded 0a32a38
• 💚 Build #121859 succeeded 7e328bc

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

[kertal]

If we can quickly look into the react performance and figure there's an easy win (not sure how often that's rendered, maybe React.memo will help us here) we can still make it, but I am also fine merging it as it is.

In this case React.memo won't help, it's rendered in every row, and usually docs are are not equal unless you've got a set of duplicate docs (but also then you would have to check it with isEqual), so I'll plead for merging it as it is. security > performance in this case

[kertal]

Code LGTM 👍 , didn't test again, but there were no changes to my first test. The performance regression is acceptable, security > performance in this case!

[kibanamachine]

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.