-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Replace lodash templates with static react renderer for field formatters #96048
Replace lodash templates with static react renderer for field formatters #96048
Conversation
{defPairs.map((pair, idx) => ( | ||
<Fragment key={idx}> | ||
<dt | ||
dangerouslySetInnerHTML={{ __html: `${pair[0]}:` }} // eslint-disable-line react/no-danger |
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.
Looking at this one again, I don't think it should be dangerously rendered, unless I call escape
. What is your preference / what is the expected behavior?
@elastic/kibana-app-services how can I test the |
💚 Build Succeeded
Metrics [docs]Page load bundle
History
To update your PR or re-run it, just comment with: |
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.
changes lgtm and seem to work well BUT - please verify that the url formatters are working correctly. I think its working but I did see it fail as well. I think it was a build problem so I'd like a second pair of eyes to verify.
@mattkime I'll verify url formatters on Monday. I had auto-merge turned on for this PR, otherwise I'd have waited to merge until verification. Sorry about that! |
Summary
Replaces the rendering logic for the
source
andcolor
field formatters. Previous implementation useslodash.template
, which requires unsafe code execution (unsafe-eval
in CSP parlance). New implementation uses ReactDOM to render static markup.Resolves #94627