Ti plugin reorg

[PhilippeOberti]

Summary

This PR does a complete restructure of the Threat Intelligence plugin. We can use this draft PR to discuss patterns that we like and ones that we do not.

I'm suggesting to not necessarily looking at the commits themselves as the changes are massive. An easier way might be to pull down the branch and look at the structure and naming and comment on that?

Here an unordered non-exhaustive list of the modifications:

• remove Indicators/Indicator in the folders names and file names
• consolidate flyout-related components under flyout folder, and remove Flyout from folder names and file names
• consolidate barchart-related components and hooks under barchart folder, and remove Barchart from folder names and file names
• consolidate table-related components, contexts and hooks under table folder
• add missing index.ts files and rename all index.tsx files to index.ts for convention and consistency
• move context-related code to contexts folders
• move translations into translations.ts files
• move all common, components and container files under public folder
• rename lib folders to utils
• ran Optimize imports command to clean things up

Random notes & questions:

• this new structure introduces a structure with more nested folders. On one hand it creates a structure a bit more feature-oriented, where all the components, hooks, interfaces... related to a feature (the definition of feature isn't clear) are in the same folder. On the other hand, for some people it can make navigating the plugin's code more difficult
• do we want to add index.ts files everywhere or just in containers and component folders?

[kibana-ci]

:broken_heart: Build Failed

• Buildkite Build
• Commit: 9031bc25e95886286d70a23060c99a6b38096a6d
• Interpreting CI Failures
• Storybooks Preview

Failed CI Steps

• Jest Tests #6
• FTR Configs #19
• FTR Configs #19
• Linting
• Check Types

Test Failures

• [job] [logs] FTR Configs #19 / apis management index lifecycle management nodes list should return the list of ES node for each custom attributes
• [job] [logs] FTR Configs #19 / apis management index lifecycle management nodes list should return the list of ES node for each custom attributes
• [job] [logs] Jest Tests #6 / useToolbarOptions() should return correct value for 25 indicators total
• [job] [logs] Jest Tests #6 / useToolbarOptions() should return correct value for 50 indicators total

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
threatIntelligence	199	221	+22

Async chunks

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

id	before	after	diff
threatIntelligence	73.1KB	73.1KB	+65.0B

Page load bundle

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

id	before	after	diff
threatIntelligence	28.6KB	28.6KB	+10.0B

Unknown metric groups

ESLint disabled in files

id	before	after	diff
apm	14	13	-1

ESLint disabled line counts

id	before	after	diff
apm	81	79	-2
securitySolution	407	403	-4
threatIntelligence	13	10	-3
total			-9

Total ESLint disabled count

id	before	after	diff
apm	95	92	-3
securitySolution	478	474	-4
threatIntelligence	14	11	-3
total			-10

History

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

[maxcold]

My 5 cents:

• do we need a separate common at all? I think having it on the same level with components, containers etc. is confusing as everything that is outside of modules is common. I think we can just move mocks, test, utils one level higher, and navigation can go into modules. The other option is actually moving everything except modules inside common, so there are only two paths public/common and public/modules as public/common inside has the same structure as any module potentially.
• barchart in utils seems alien after introducing barchart inside modules/indicators
• about deep nesting, I also have mixed feelings about it. It looks logical but I don't like the duality of components. component can mean either a stateless component or a "submodule" now. But I don't have a good solution for that tbh. Another problem I see is that lifting up should be simpler with the deep nesting, but lifting down is probably harder as you don't have good visibility into what is going on deep. Eg. if there is a hook inside indicators/hooks which is related to the field browser, it is quite hard to grasp that there might also be indicator/components/table/components/field_browser/hooks.

While writing it I also realized that maybe trying to place "common" things under one umbrella will lead to a pile of smth anyway. And in the end, everything can be part of a bigger logical module. I see the following patterns:

• components/date_formatter and utils/dates could go into modules/dates
• mocks/mock_kibana_ui, containers/security_solution*, hooks/use_kibana*, hooks/use_security_context could go into modules/kibana_interop or maybe into two + modules/security_solution_interop
• components/paywall and containers/enterprise_guard could go into modules/enterprise_guard
• mocks/story_provides could go into `modules/storybook/story_providers
• Stateless components agnostic to modules they are used in probably should be a module itself, eg. layout.

I tried to prepare a branch where I moved things around to POC with this approach. Maybe as with many ideas, it looks smart but disastrous in the end, but I'd like to hear your thoughts https://github.com/elastic/kibana/pull/141380

Also we should always remind ourselves about this good advice from react docs https://reactjs.org/docs/faq-structure.html#dont-overthink-it :)

[lgestc]

nice job, though I have not went through it all yet. I will!

[PhilippeOberti]

thanks @maxcold! Super useful to have your branch, it shows me things I didn't think about. It's nice to have other approaches as I find it often difficult to see outside of an idea you have...

Couple of comments:

• why not have a test module. It feels a bit weird to have only 2 top-level folders, and I feel like if we have a storybook module then we could have a test module
• if we move the test folder in modules, then why not getting rid of that modules folder entirely and have all the folders inside at the top level, right under public? (this is how the Security Solution plugin is)
• one thing that I'm not a fan of though is that in modules, we now have folders that represents sections of the plugin (like indicators, timeline and query for example, some other that are just groups of components (like dates or eui_interop). The same way cypress is a separate folder, I feel like I would like the idea of having things like tests, mocks, storybook utils or utils in general outisde of the notion of modules
• as a specific example I find it weird to have mock_security_context under kibana_interop but it's actually only used in the test and storybook_interop folders...

I'm not going to spend more time on this for now. I feel like I want the idea to mature in my head a bit, and discussing more all together over time will I'm sure bring more good ideas! :)