Skip to content
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

[Fleet] Improve Egonomics and Clarity for Imports #100566

Open
kpollich opened this issue May 25, 2021 · 1 comment
Open

[Fleet] Improve Egonomics and Clarity for Imports #100566

kpollich opened this issue May 25, 2021 · 1 comment
Labels
Team:Fleet Team label for Observability Data Collection Fleet team technical debt Improvement of the software architecture and operational architecture

Comments

@kpollich
Copy link
Member

Wanted to call out two specific areas for improvements for how we import modules in the Fleet codebase:

  1. Multiple imports from directories with the same name
  2. "Long-reaching" imports

1. Multiple imports from directories with the same name

It's common in the Fleet codebase that we import components from a few locations, e.g.

  • "Local" components, e.g. components related to the current screen/feature/module
  • "Parent" components, e.g. shared components for a section like EPM or Agent Policies
  • "Common" components, e.g. components that appear globally across the entire Fleet application

When two or more of these imports occur in same file, it can introduce some confusion around "what components come from where", e.g:

import { Loading } from '../../../components';
import { ConfirmDeployAgentPolicyModal } from '../components';
import { useIntraAppState } from '../../../hooks/use_intra_app_state';
import { useUIExtension } from '../../../hooks/use_ui_extension';
import { ExtensionWrapper } from '../../../components/extension_wrapper';
import type { PackagePolicyEditExtensionComponentProps } from '../../../types';
import { PLUGIN_ID } from '../../../../../../common/constants';
import { pkgKeyFromPackageInfo } from '../../../services/pkg_key_from_package_info';
import { CreatePackagePolicyPageLayout } from './components';
import type { CreatePackagePolicyFrom, PackagePolicyFormState } from './types';
import type { PackagePolicyValidationResults } from './services';
import { validatePackagePolicy, validationHasErrors } from './services';
import { StepSelectPackage } from './step_select_package';
import { StepSelectAgentPolicy } from './step_select_agent_policy';
import { StepConfigurePackagePolicy } from './step_configure_package';
import { StepDefinePackagePolicy } from './step_define_package_policy';

In the above example, we have the following component imports:

// "Common"
import { Loading } from '../../../components';

// "Parent"
import { ConfirmDeployAgentPolicyModal } from '../components';

// "Common", but from a specific path
import { ExtensionWrapper } from '../../../components/extension_wrapper';

// "Local"
import { CreatePackagePolicyPageLayout } from './components';

We might be able to improve clarity around these imports by using unique pathnames other than components to denote shared/common components, but there are likely other improvements we can make as well.

For example, we could consider the following changes to the above imports:

// Group common components - not sure if ESLint interferes here
import { Loading } from '../../../common/components';
import { ExtensionWrapper } from '../../../common/components/extension_wrapper';

// Introduce another directory to clarify when components are shared for a given section/feature/set of screens, etc
import { ConfirmDeployAgentPolicyModal } from '../shared/components';

// Leave "Local" components as-is
import { CreatePackagePolicyPageLayout } from './components';

This is something I came across a lot when moving files around during the implementation of #99848.

2. "Long-reaching" imports

We have a number of places - most notably when we import from common - where imports come from many directories above a given module. For example, in the same file as above:

This seems to be mitigated by the convention of placing "checkpoint" modules that re-export from parent directories further down the chain, which act largely as a way to limit the number of directories we need to reach into, although they can also export application-level modules as well.

These kinds of noisy imports could potentially be resolved without the need for these intermediary "re-export" modules via TypeScript's path mapping feature, but that may require higher-level configuration changes in Kibana's TypeScript setup.

@kpollich kpollich added technical debt Improvement of the software architecture and operational architecture Team:Fleet Team label for Observability Data Collection Fleet team labels May 25, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Fleet Team label for Observability Data Collection Fleet team technical debt Improvement of the software architecture and operational architecture
Projects
None yet
Development

No branches or pull requests

2 participants