Skip to content

Commit

Permalink
Add eslint rule to support breaking up packages (#130483)
Browse files Browse the repository at this point in the history
  • Loading branch information
Spencer authored Apr 18, 2022
1 parent 34dfeeb commit 20f05e2
Show file tree
Hide file tree
Showing 10 changed files with 786 additions and 31 deletions.
1 change: 1 addition & 0 deletions packages/elastic-eslint-config-kibana/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ NPM_MODULE_EXTRA_FILES = [
RUNTIME_DEPS = [
"//packages/kbn-babel-preset",
"//packages/kbn-dev-utils",
"//packages/kbn-eslint-plugin-imports",
"@npm//eslint-config-prettier",
"@npm//semver",
]
Expand Down
1 change: 1 addition & 0 deletions packages/kbn-eslint-plugin-imports/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ TYPES_DEPS = [
"//packages/kbn-utils:npm_module_types",
"//packages/kbn-dev-utils:npm_module_types", # only required for the tests, which are excluded except on windows
"//packages/kbn-import-resolver:npm_module_types",
"@npm//dedent", # only required for the tests, which are excluded except on windows
"@npm//@types/eslint",
"@npm//@types/jest",
"@npm//@types/node",
Expand Down
55 changes: 51 additions & 4 deletions packages/kbn-eslint-plugin-imports/README.md
Original file line number Diff line number Diff line change
@@ -1,9 +1,56 @@
# @kbn/eslint-plugin-imports

ESLint plugin providing custom rules for validating imports in the Kibana repo with custom logic beyond what's possible with custom config to eslint-plugin-imports and even a custom resolver
ESLint plugin providing custom rules for validating imports in the Kibana repo with custom logic beyond what's possible with custom config to eslint-plugin-imports and even a custom resolver.

## `resolveKibanaImport(request: string, dirname: string)`
For the purposes of this ESLint plugin "imports" include:

Resolve an import request (the "from" string from an import statement, or any other relative/absolute import path) from a given directory. The `dirname` should be the same for all files in a given directory.
- `import` statements
- `import()` expressions
- `export ... from` statements
- `require()` calls
- `require.resolve()` calls
- `jest.mock()` and related calls

Result will be `null` when the import path does not resolve, but all valid/committed import paths *should* resolve. Other result values are documented in src/resolve_result.ts.
An "import request" is the string defining the target package/module by any of the previous mentioned "import" types

## `@kbn/imports/no_unresolvable_imports`

This rule validates that every import request in the repository can be resolved by `@kbn/import-resolver`.

This rule is not configurable, should never be skipped, and is auto-fixable.

If a valid import request can't be resolved for some reason please reach out to Kibana Operations to work on either a different strategy for the import or help updating the resolve to support the new import strategy.

## `@kbn/imports/uniform_imports`

This rule validates that every import request in the repsitory follows a standard set of formatting rules. See the rule implemeation for a full breakdown but here is a breif summary:

- imports within a single package must use relative paths
- imports across packages must reference the other package using it's module id
- imports to code not in a package must use relative paths
- imports to an `index` file end with the directory name, ie `/index` or `/index.{ext}` are stripped
- unless this is a `require.resolve()`, the imports should not mention file extensions. `require.resolve()` calls will retain the extension if added manually

This rule is not configurable, should never be skipped, and is auto-fixable.

## `@kbn/imports/exports_moved_packages`

This rule assists package authors who are doing the good work of breaking up large packages. The goal is to define exports which used to be part of one package as having moved to another package. The configuration maintains this mapping and is designed to be extended in the future is additional needs arrise like targetting specific package types.

Config example:
```ts
'@kbn/imports/exports_moved_packages': ['error', [
{
fromPackage: '@kbn/kitchen-sink',
toPackage: '@kbn/spatula',
exportNames: [
'Spatula',
'isSpatula'
]
}
]]
```

This config will find any import of `@kbn/kitchen-sink` which specifically references the `Spatula` or `isSpatula` exports, remove the old exports from the import (potentially removing the entire import), and add a new import after the previous following it's style pointing to the new package.

The auto-fixer here covers the vast majority of import styles in the repository but might not cover everything, including `import * as Namespace from '@kbn/kitchen-sink'`. Imports like this will need to be found and updated manually, though TypeScript should be able to find the vast majority of those.
13 changes: 13 additions & 0 deletions packages/kbn-eslint-plugin-imports/jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

module.exports = {
preset: '@kbn/test/jest_node',
rootDir: '../..',
roots: ['<rootDir>/packages/kbn-eslint-plugin-imports'],
};
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,27 @@ const JEST_MODULE_METHODS = [
'jest.requireMock',
];

export type Importer =
| TSESTree.ImportDeclaration
| T.ImportDeclaration
| TSESTree.ExportNamedDeclaration
| T.ExportNamedDeclaration
| TSESTree.ExportAllDeclaration
| T.ExportAllDeclaration
| TSESTree.CallExpression
| T.CallExpression
| TSESTree.ImportExpression
| TSESTree.CallExpression
| T.CallExpression;

export type SomeNode = TSESTree.Node | T.Node;

type Visitor = (req: string | null, node: SomeNode, type: ImportType) => void;
interface VisitorContext {
node: SomeNode;
type: ImportType;
importer: Importer;
}
type Visitor = (req: string | null, context: VisitorContext) => void;

const isIdent = (node: SomeNode): node is TSESTree.Identifier | T.Identifier =>
T.isIdentifier(node) || node.type === AST_NODE_TYPES.Identifier;
Expand All @@ -36,28 +54,38 @@ const isStringLiteral = (node: SomeNode): node is TSESTree.StringLiteral | T.Str
const isTemplateLiteral = (node: SomeNode): node is TSESTree.TemplateLiteral | T.TemplateLiteral =>
T.isTemplateLiteral(node) || node.type === AST_NODE_TYPES.TemplateLiteral;

function passSourceAsString(source: SomeNode | null | undefined, type: ImportType, fn: Visitor) {
if (!source) {
function passSourceAsString(
fn: Visitor,
node: SomeNode | null | undefined,
importer: Importer,
type: ImportType
) {
if (!node) {
return;
}

if (isStringLiteral(source)) {
return fn(source.value, source, type);
const ctx = {
node,
importer,
type,
};

if (isStringLiteral(node)) {
return fn(node.value, ctx);
}

if (isTemplateLiteral(source)) {
if (source.expressions.length) {
if (isTemplateLiteral(node)) {
if (node.expressions.length) {
return null;
}

return fn(
[...source.quasis].reduce((acc, q) => acc + q.value.raw, ''),
source,
type
[...node.quasis].reduce((acc, q) => acc + q.value.raw, ''),
ctx
);
}

return fn(null, source, type);
return fn(null, ctx);
}

/**
Expand All @@ -68,27 +96,28 @@ function passSourceAsString(source: SomeNode | null | undefined, type: ImportTyp
export function visitAllImportStatements(fn: Visitor) {
const visitor = {
ImportDeclaration(node: TSESTree.ImportDeclaration | T.ImportDeclaration) {
passSourceAsString(node.source, 'esm', fn);
passSourceAsString(fn, node.source, node, 'esm');
},
ExportNamedDeclaration(node: TSESTree.ExportNamedDeclaration | T.ExportNamedDeclaration) {
passSourceAsString(node.source, 'esm', fn);
passSourceAsString(fn, node.source, node, 'esm');
},
ExportAllDeclaration(node: TSESTree.ExportAllDeclaration | T.ExportAllDeclaration) {
passSourceAsString(node.source, 'esm', fn);
passSourceAsString(fn, node.source, node, 'esm');
},
ImportExpression(node: TSESTree.ImportExpression) {
passSourceAsString(node.source, 'esm', fn);
passSourceAsString(fn, node.source, node, 'esm');
},
CallExpression({ callee, arguments: args }: TSESTree.CallExpression | T.CallExpression) {
CallExpression(node: TSESTree.CallExpression | T.CallExpression) {
const { callee, arguments: args } = node;
// babel parser used for .js files treats import() calls as CallExpressions with callees of type "Import"
if (T.isImport(callee)) {
passSourceAsString(args[0], 'esm', fn);
passSourceAsString(fn, args[0], node, 'esm');
return;
}

// is this a `require()` call?
if (isIdent(callee) && callee.name === 'require') {
passSourceAsString(args[0], 'require', fn);
passSourceAsString(fn, args[0], node, 'require');
return;
}

Expand All @@ -103,12 +132,12 @@ export function visitAllImportStatements(fn: Visitor) {

// is it "require.resolve()"?
if (name === 'require.resolve') {
passSourceAsString(args[0], 'require-resolve', fn);
passSourceAsString(fn, args[0], node, 'require-resolve');
}

// is it one of jest's mock methods?
if (left.name === 'jest' && JEST_MODULE_METHODS.includes(name)) {
passSourceAsString(args[0], 'jest', fn);
passSourceAsString(fn, args[0], node, 'jest');
}
}
},
Expand Down
2 changes: 2 additions & 0 deletions packages/kbn-eslint-plugin-imports/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
export * from './get_import_resolver';
import { NoUnresolvableImportsRule } from './rules/no_unresolvable_imports';
import { UniformImportsRule } from './rules/uniform_imports';
import { ExportsMovedPackagesRule } from './rules/exports_moved_packages';

/**
* Custom ESLint rules, add `'@kbn/eslint-plugin-imports'` to your eslint config to use them
Expand All @@ -17,4 +18,5 @@ import { UniformImportsRule } from './rules/uniform_imports';
export const rules = {
no_unresolvable_imports: NoUnresolvableImportsRule,
uniform_imports: UniformImportsRule,
exports_moved_packages: ExportsMovedPackagesRule,
};
Loading

0 comments on commit 20f05e2

Please sign in to comment.