-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Alerting UI] Reduced triggersActionsUi bundle size by making all action types UI validation messages translations asynchronous. #100525
Conversation
…nectors validation messages translations asyncronus.
Pinging @elastic/kibana-alerting-services (Team:Alerting Services) |
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.
Looks good overall. A few nits about creating reusable variables.
Also saw this after adding a pagerduty action to an alert and then editing the alert:
Uncaught TypeError: Cannot read property 'length' of undefined
at PagerDutyParamsFields (pagerduty_params.tsx:138)
at renderWithHooks (react-dom.development.js:16260)
at updateFunctionComponent (react-dom.development.js:18347)
at mountLazyComponent (react-dom.development.js:18657)
at beginWork$1 (react-dom.development.js:20168)
at HTMLUnknownElement.callCallback (react-dom.development.js:336)
at Object.invokeGuardedCallbackDev (react-dom.development.js:385)
at invokeGuardedCallback (react-dom.development.js:440)
at beginWork$$1 (react-dom.development.js:25780)
at performUnitOfWork (react-dom.development.js:24698)
PagerDutyParamsFields @ pagerduty_params.tsx:138
renderWithHooks @ react-dom.development.js:16260
updateFunctionComponent @ react-dom.development.js:18347
mountLazyComponent @ react-dom.development.js:18657
beginWork$1 @ react-dom.development.js:20168
callCallback @ react-dom.development.js:336
invokeGuardedCallbackDev @ react-dom.development.js:385
invokeGuardedCallback @ react-dom.development.js:440
beginWork$$1 @ react-dom.development.js:25780
performUnitOfWork @ react-dom.development.js:24698
workLoopSync @ react-dom.development.js:24671
performSyncWorkOnRoot @ react-dom.development.js:24270
(anonymous) @ react-dom.development.js:12199
unstable_runWithPriority @ scheduler.development.js:697
runWithPriority$2 @ react-dom.development.js:12149
flushSyncCallbackQueueImpl @ react-dom.development.js:12194
flushSyncCallbackQueue @ react-dom.development.js:12182
scheduleUpdateOnFiber @ react-dom.development.js:23709
dispatchAction @ react-dom.development.js:17076
(anonymous) @ action_form.tsx:96
async function (async)
(anonymous) @ action_form.tsx:85
(anonymous) @ action_form.tsx:98
commitHookEffectList @ react-dom.development.js:22030
commitPassiveHookEffects @ react-dom.development.js:22064
callCallback @ react-dom.development.js:336
invokeGuardedCallbackDev @ react-dom.development.js:385
invokeGuardedCallback @ react-dom.development.js:440
flushPassiveEffectsImpl @ react-dom.development.js:25392
unstable_runWithPriority @ scheduler.development.js:697
runWithPriority$2 @ react-dom.development.js:12149
flushPassiveEffects @ react-dom.development.js:25361
performSyncWorkOnRoot @ react-dom.development.js:24251
(anonymous) @ react-dom.development.js:12199
unstable_runWithPriority @ scheduler.development.js:697
runWithPriority$2 @ react-dom.development.js:12149
flushSyncCallbackQueueImpl @ react-dom.development.js:12194
flushSyncCallbackQueue @ react-dom.development.js:12182
scheduleUpdateOnFiber @ react-dom.development.js:23709
dispatchAction @ react-dom.development.js:17076
(anonymous) @ alert_form.tsx:166
async function (async)
(anonymous) @ alert_form.tsx:151
(anonymous) @ alert_form.tsx:184
commitHookEffectList @ react-dom.development.js:22030
commitPassiveHookEffects @ react-dom.development.js:22064
callCallback @ react-dom.development.js:336
invokeGuardedCallbackDev @ react-dom.development.js:385
invokeGuardedCallback @ react-dom.development.js:440
flushPassiveEffectsImpl @ react-dom.development.js:25392
unstable_runWithPriority @ scheduler.development.js:697
runWithPriority$2 @ react-dom.development.js:12149
flushPassiveEffects @ react-dom.development.js:25361
performSyncWorkOnRoot @ react-dom.development.js:24251
(anonymous) @ react-dom.development.js:12199
unstable_runWithPriority @ scheduler.development.js:697
runWithPriority$2 @ react-dom.development.js:12149
flushSyncCallbackQueueImpl @ react-dom.development.js:12194
flushSyncCallbackQueue @ react-dom.development.js:12182
scheduleUpdateOnFiber @ react-dom.development.js:23709
dispatchAction @ react-dom.development.js:17076
(anonymous) @ health_check.tsx:52
async function (async)
(anonymous) @ health_check.tsx:36
(anonymous) @ health_check.tsx:54
commitHookEffectList @ react-dom.development.js:22030
commitPassiveHookEffects @ react-dom.development.js:22064
callCallback @ react-dom.development.js:336
invokeGuardedCallbackDev @ react-dom.development.js:385
invokeGuardedCallback @ react-dom.development.js:440
flushPassiveEffectsImpl @ react-dom.development.js:25392
unstable_runWithPriority @ scheduler.development.js:697
runWithPriority$2 @ react-dom.development.js:12149
flushPassiveEffects @ react-dom.development.js:25361
performSyncWorkOnRoot @ react-dom.development.js:24251
(anonymous) @ react-dom.development.js:12199
unstable_runWithPriority @ scheduler.development.js:697
runWithPriority$2 @ react-dom.development.js:12149
flushSyncCallbackQueueImpl @ react-dom.development.js:12194
flushSyncCallbackQueue @ react-dom.development.js:12182
discreteUpdates$1 @ react-dom.development.js:24423
discreteUpdates @ react-dom.development.js:1438
dispatchDiscreteEvent @ react-dom.development.js:5881
react-dom.development.js:21843 The above error occurred in the <PagerDutyParamsFields> component:
in PagerDutyParamsFields (created by ActionTypeForm)
in Suspense (created by ActionTypeForm)
in EuiErrorBoundary (created by ActionTypeForm)
in div (created by EuiResizeObserver)
in div (created by EuiResizeObserver)
in EuiResizeObserver (created by EuiAccordion)
in div (created by EuiAccordion)
in div (created by EuiAccordion)
in EuiAccordion (created by ActionTypeForm)
in ActionTypeForm (created by ActionForm)
in ActionForm
in Suspense
in Unknown (created by AlertForm)
in div (created by EuiForm)
in EuiForm (created by AlertForm)
in AlertForm (created by AlertEdit)
in div (created by EuiFlyoutBody)
in div (created by EuiFlyoutBody)
in div (created by EuiFlyoutBody)
in EuiFlyoutBody (created by AlertEdit)
in HealthCheck (created by AlertEdit)
in HealthContextProvider (created by AlertEdit)
in div (created by EuiFlyout)
in div (created by ForwardRef)
in ForwardRef (created by ForwardRef)
in ForwardRef (created by ForwardRef)
in ForwardRef (created by ForwardRef)
in ForwardRef (created by EuiFocusTrap)
in EuiFocusTrap (created by EuiFlyout)
in EuiFlyout (created by AlertEdit)
in EuiPortal (created by AlertEdit)
in AlertEdit
in Suspense
...gers_actions_ui/public/application/components/builtin_action_types/email/email_connector.tsx
Outdated
Show resolved
Hide resolved
...riggers_actions_ui/public/application/components/builtin_action_types/email/email_params.tsx
Outdated
Show resolved
Hide resolved
...s_actions_ui/public/application/components/builtin_action_types/es_index/es_index_params.tsx
Outdated
Show resolved
Hide resolved
...actions_ui/public/application/components/builtin_action_types/webhook/webhook_connectors.tsx
Outdated
Show resolved
Hide resolved
@ymao1 thank you for a so careful review! |
@elasticmachine merge upstream |
...ck/plugins/security_solution/public/detections/components/rules/step_rule_actions/schema.tsx
Show resolved
Hide resolved
@YulNaumenko I am still seeing this. To recreate, I start creating a new rule, add an |
This is not related to the current PR, so I've opened a bug for fixing this. |
@elasticmachine merge upstream |
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.
LGTM
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.
Security solution changes LTGM!
@elasticmachine merge upstream |
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.
Stack Monitoring changes look good 👍
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 though I've noticed when the translations file takes time to load (async call), the validation messages don't appear until then. Is slow internet connections something to be concerned about for this UX?
@mikecote, thank you. Good point. Do you think some kind of spinner(progress bar) in the UI could help? Just to show the user that something still happening. |
@elastic/kibana-core could someone clarify if we should worry about a bad UX for a slowly loaded bundle chunks, in the current case it is translations for UI validation messages? |
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]Module Count
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: cc @YulNaumenko |
…ion types UI validation messages translations asynchronous. (elastic#100525) * [Alerting UI] Reduced triggersActionsUi bundle size by making all connectors validation messages translations asyncronus. * changed validation logic to be async * fixed action form * fixed tests * fixed tests * fixed validation usage in security * fixed due to comments * fixed due to comments * added spinner for the validation awaiting * fixed typechecks Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
…ion types UI validation messages translations asynchronous. (#100525) (#101240) * [Alerting UI] Reduced triggersActionsUi bundle size by making all connectors validation messages translations asyncronus. * changed validation logic to be async * fixed action form * fixed tests * fixed tests * fixed validation usage in security * fixed due to comments * fixed due to comments * added spinner for the validation awaiting * fixed typechecks Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Resolves #95871
Summary
Changed action type validation methods validateConnector and validateParams to be asynchronous and load translations only when it's executed.
For the delayed translation loading (example simulate 5000ms delay) we decided to show the spinner:
Screen.Recording.2021-06-02.at.12.27.28.PM.mov