[Security Solution] [Cases] Various Cases cleanups#102103
[Security Solution] [Cases] Various Cases cleanups#102103stephmilovic merged 27 commits intoelastic:masterfrom
Conversation
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
|
Pinging @elastic/security-threat-hunting (Team:Threat Hunting) |
| files: ['x-pack/plugins/cases/**/*.{js,mjs,ts,tsx}'], | ||
| rules: { | ||
| 'arrow-body-style': ['warn', 'as-needed'], | ||
| 'no-duplicate-imports': 'error', |
| block: jest.fn(), | ||
| createHref: jest.fn(), | ||
| listen: jest.fn(), | ||
| push: () => {}, |
There was a problem hiding this comment.
Do we still have some tests using this mock now that we removed them from the file below?
There was a problem hiding this comment.
good point no i dont think so, there is one but its import routeData from 'react-router';. ill take care of it
x-pack/plugins/cases/public/components/user_action_tree/index.test.tsx
Outdated
Show resolved
Hide resolved
| useEffect(() => { | ||
| return () => { | ||
| useEffect( | ||
| () => () => { |
There was a problem hiding this comment.
nit: this is probably just me but for this specific instance I find this harder to read 🤷♂️
| export interface CasesNavigation<T = React.MouseEvent | MouseEvent | null, K = null> { | ||
| href: K extends 'configurable' ? (arg: T) => string : string; | ||
| onClick: (arg: T) => void; | ||
| onClick: K extends 'configurable' |
There was a problem hiding this comment.
Should we also add void | Promise<void> since in some cases this can be an async function?
x-pack/plugins/security_solution/public/cases/components/case_view/index.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/public/cases/components/case_view/index.tsx
Outdated
Show resolved
Hide resolved
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
Summary
Various Cases cleanups. Some for performance reasons, other for preference to shorten code:
useHistoryfromreact-router-domto usenavigateToApp, and removedRouter/mockHistoryfrom tests <- this is probably the most important piece to reviewuseMemo/useCallbackwhere it should've been in the first place"arrow-body-style": ["error", "as-needed"]and then removed because I realized prettier will cancel the rule out. I left in the commit with 23 file changes just because I struggled to remove the commit. sorry about that reviewer.¯\_(ツ)_/¯The big change that made our
limits.ymlsize go down was removing unused translations. I am going to be making more performance related changes in a future PR, especially around translations, but wanted to post what I had so far in a PR since it touches a lot already.