Skip to content

[eslint] fix and skip violations for cross-boundary imports #136911

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.

Sign up for GitHub

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

Merged
spalger merged 16 commits into from
Jul 29, 2022
Merged

[eslint] fix and skip violations for cross-boundary imports #136911

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
2cedce2
enable @kbn/imports/no_boundary_crossing rule
Jul 26, 2022
460ebe8
fix classification of canvas/functions/external
Jul 26, 2022
76944d2
rename "scripts" dir to "cli"
Jul 26, 2022
d7e5255
fix imports to use `import type` or import correct module
Jul 26, 2022
3cb6f47
Skip violations that need to be fixed with codeowners
Jul 26, 2022
b10d5f0
remove @kbn/eslint/no-restricted-paths rule
Jul 26, 2022
9763015
Merge branch 'main' of github.com:elastic/kibana into implement/preve…
Jul 27, 2022
4426bf8
Merge branch 'main' into implement/prevent-bad-cross-boundary-imports
Jul 27, 2022
aa80ec8
Merge branch 'main' into implement/prevent-bad-cross-boundary-imports
Jul 27, 2022
a80fe93
Merge branch 'main' into implement/prevent-bad-cross-boundary-imports
Jul 28, 2022
2e813bc
Merge branch 'main' of github.com:elastic/kibana into implement/preve…
Jul 28, 2022
d5589a8
Merge branch 'main' into implement/prevent-bad-cross-boundary-imports
Jul 28, 2022
f0bf6f2
Merge branch 'main' of github.com:elastic/kibana into implement/preve…
Jul 29, 2022
c9c30ef
switch to "import type" in some ml modules
Jul 29, 2022
4faf899
Merge branch 'main' into implement/prevent-bad-cross-boundary-imports
Jul 29, 2022
b8eda14
Merge branch 'main' into implement/prevent-bad-cross-boundary-imports
Jul 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
103 changes: 0 additions & 103 deletions .eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -585,109 +585,6 @@ module.exports = {
},
},

/**
* Restricted paths
*/
{
files: ['**/*.{js,mjs,ts,tsx}'],
rules: {
'@kbn/eslint/no-restricted-paths': [
'error',
{
basePath: __dirname,
zones: [
{
target: ['(src|x-pack)/**/*', '!src/core/**/*'],
from: ['src/core/utils/**/*'],
errorMessage: `Plugins may only import from src/core/server and src/core/public.`,
},
{
target: ['(src|x-pack)/plugins/*/server/**/*'],
from: ['(src|x-pack)/plugins/*/public/**/*'],
errorMessage: `Server code can not import from public, use a common directory.`,
},
{
target: ['(src|x-pack)/plugins/*/common/**/*'],
from: ['(src|x-pack)/plugins/*/(server|public)/**/*'],
errorMessage: `Common code can not import from server or public, use a common directory.`,
},
{
target: ['(src|x-pack)/plugins/**/(public|server)/**/*', 'examples/**/*'],
from: [
'src/core/public/**/*',
'!src/core/public/index.ts', // relative import
'!src/core/public/mocks{,.ts}',
'!src/core/server/types{,.ts}',
'!src/core/public/utils/**/*',
'!src/core/public/*.test.mocks{,.ts}',

'src/core/server/**/*',
'!src/core/server/index.ts', // relative import
'!src/core/server/mocks{,.ts}',
'!src/core/server/types{,.ts}',
'!src/core/server/test_utils{,.ts}',
// for absolute imports until fixed in
// https://github.com/elastic/kibana/issues/36096
'!src/core/server/*.test.mocks{,.ts}',

'target/types/**',
],
allowSameFolder: true,
errorMessage:
'Plugins may only import from top-level public and server modules in core.',
},
{
target: [
'(src|x-pack)/plugins/**/(public|server)/**/*',
'examples/**/*',
'!(src|x-pack)/**/*.test.*',
'!(x-pack/)?test/**/*',
],
from: [
'(src|x-pack)/plugins/**/(public|server)/**/*',
'!(src|x-pack)/plugins/**/(public|server)/mocks/index.{js,mjs,ts}',
'!(src|x-pack)/plugins/**/(public|server)/(index|mocks).{js,mjs,ts,tsx}',
'!(src|x-pack)/plugins/**/__stories__/index.{js,mjs,ts,tsx}',
'!(src|x-pack)/plugins/**/__fixtures__/index.{js,mjs,ts,tsx}',
],
allowSameFolder: true,
errorMessage: 'Plugins may only import from top-level public and server modules.',
},
{
target: [
'(src|x-pack)/plugins/**/*',
'!(src|x-pack)/plugins/**/server/**/*',

'examples/**/*',
'!examples/**/server/**/*',
],
from: [
'src/core/server',
'src/core/server/**/*',
'(src|x-pack)/plugins/*/server/**/*',
'examples/**/server/**/*',
// TODO: Remove the 'joi' eslint rule once IE11 support is dropped
'joi',
],
errorMessage:
'Server modules cannot be imported into client modules or shared modules.',
},
{
target: ['src/core/**/*'],
from: ['plugins/**/*', 'src/plugins/**/*'],
errorMessage: 'The core cannot depend on any plugins.',
},
{
target: ['(src|x-pack)/plugins/*/public/**/*'],
from: ['ui/**/*'],
errorMessage: 'Plugins cannot import legacy UI code.',
},
],
},
],
},
},

/**
* Allow default exports
*/
Expand Down
2 changes: 1 addition & 1 deletion examples/embeddable_examples/common/book_saved_object_attributes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/

import { SavedObjectAttributes } from '@kbn/core/types';
import type { SavedObjectAttributes } from '@kbn/core/types';

export const BOOK_SAVED_OBJECT = 'book';

Expand Down
2 changes: 1 addition & 1 deletion examples/embeddable_examples/common/todo_saved_object_attributes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/

import { SavedObjectAttributes } from '@kbn/core/types';
import type { SavedObjectAttributes } from '@kbn/core/types';

export interface TodoSavedObjectAttributes extends SavedObjectAttributes {
task: string;
Expand Down
2 changes: 0 additions & 2 deletions examples/field_formats_example/public/app.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,8 @@ import example1SampleCode from '!!raw-loader!./examples/1_using_existing_format'
import example2SampleCodePart1 from '!!raw-loader!../common/example_currency_format';
// @ts-ignore
import example2SampleCodePart2 from '!!raw-loader!./examples/2_creating_custom_formatter';
/* eslint-disable @kbn/eslint/no-restricted-paths */
// @ts-ignore
import example2SampleCodePart3 from '!!raw-loader!../server/examples/2_creating_custom_formatter';
/* eslint-enable @kbn/eslint/no-restricted-paths */
// @ts-ignore
import example3SampleCode from '!!raw-loader!./examples/3_creating_custom_format_editor';

Expand Down
1 change: 1 addition & 0 deletions packages/core/config/core-config-server-internal/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@

export { coreDeprecationProvider } from './deprecation';
export { ensureValidConfiguration } from './ensure_valid_configuration';
// eslint-disable-next-line @kbn/imports/no_boundary_crossing
Copy link
Copy Markdown
Contributor

@TinaHeiligers TinaHeiligers Aug 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@spalger how would we resolve this?

Copy link
Copy Markdown
Contributor Author

@spalger spalger Aug 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem here is that test_utils are assumed to be code only intended for use by tests, but it's imported from the index.ts file of the package, meaning that it's included in the distributable and these test helpers are exposed to everyone who imports this package. If these helpers are actually used in other packages then we will need to move them to a new test-helpers package (though we can't define the type of the package yet, that would be it's purpose and it's name should reflect that somehow). Then we can import the @kbn/core-config-server-test-helpers package in the local tests and in the other packages which use them.

If these test helpers are not actually needed outside this package then this export ... from statement can just be removed.

export { getDeprecationsFor, getDeprecationsForGlobalSettings } from './test_utils';
2 changes: 1 addition & 1 deletion packages/elastic-apm-synthtrace/src/cli.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@
* Side Public License, v 1.
*/

export { runSynthtrace } from './scripts/run_synthtrace';
export { runSynthtrace } from './cli/run_synthtrace';
Copy link
Copy Markdown
Contributor Author

@spalger spalger Jul 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"scripts" directories are used to store tooling files, which are only necessary for local development and not a part of package source code. I hope this rename is acceptable

0 ...-synthtrace/src/scripts/run_synthtrace.ts → ...-apm-synthtrace/src/cli/run_synthtrace.ts
View file
Open in desktop
File renamed without changes.
0 ...ic-apm-synthtrace/src/scripts/scenario.ts → ...lastic-apm-synthtrace/src/cli/scenario.ts
View file
Open in desktop
File renamed without changes.
0 .../src/scripts/utils/get_common_services.ts → ...race/src/cli/utils/get_common_services.ts
View file
Open in desktop
File renamed without changes.
0 ...thtrace/src/scripts/utils/get_scenario.ts → ...-synthtrace/src/cli/utils/get_scenario.ts
View file
Open in desktop
File renamed without changes.
0 ...trace/src/scripts/utils/interval_to_ms.ts → ...ynthtrace/src/cli/utils/interval_to_ms.ts
View file
Open in desktop
File renamed without changes.
0 .../src/scripts/utils/parse_run_cli_flags.ts → ...race/src/cli/utils/parse_run_cli_flags.ts
View file
Open in desktop
File renamed without changes.
0 ...pts/utils/start_historical_data_upload.ts → ...cli/utils/start_historical_data_upload.ts
View file
Open in desktop
File renamed without changes.
0 ...c/scripts/utils/start_live_data_upload.ts → ...e/src/cli/utils/start_live_data_upload.ts
View file
Open in desktop
File renamed without changes.
0 ...ce/src/scripts/utils/synthtrace_worker.ts → ...htrace/src/cli/utils/synthtrace_worker.ts
View file
Open in desktop
File renamed without changes.
0 ...pm-synthtrace/src/scripts/utils/worker.js → ...ic-apm-synthtrace/src/cli/utils/worker.js
View file
Open in desktop
File renamed without changes.
6 changes: 3 additions & 3 deletions packages/elastic-apm-synthtrace/src/scenarios/agent_config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
*/

import { observer, timerange } from '..';
import { Scenario } from '../scripts/scenario';
import { getLogger } from '../scripts/utils/get_common_services';
import { RunOptions } from '../scripts/utils/parse_run_cli_flags';
import { Scenario } from '../cli/scenario';
import { getLogger } from '../cli/utils/get_common_services';
import { RunOptions } from '../cli/utils/parse_run_cli_flags';
import { AgentConfigFields } from '../lib/agent_config/agent_config_fields';

const scenario: Scenario<AgentConfigFields> = async (runOptions: RunOptions) => {
Expand Down
6 changes: 3 additions & 3 deletions packages/elastic-apm-synthtrace/src/scenarios/aws_lambda.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@

import { apm, timerange } from '..';
import { ApmFields } from '../lib/apm/apm_fields';
import { Scenario } from '../scripts/scenario';
import { getLogger } from '../scripts/utils/get_common_services';
import { RunOptions } from '../scripts/utils/parse_run_cli_flags';
import { Scenario } from '../cli/scenario';
import { getLogger } from '../cli/utils/get_common_services';
import { RunOptions } from '../cli/utils/parse_run_cli_flags';

const ENVIRONMENT = __filename;

Expand Down
6 changes: 3 additions & 3 deletions packages/elastic-apm-synthtrace/src/scenarios/kibana_stats.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
*/

import { stackMonitoring, timerange } from '..';
import { Scenario } from '../scripts/scenario';
import { getLogger } from '../scripts/utils/get_common_services';
import { RunOptions } from '../scripts/utils/parse_run_cli_flags';
import { Scenario } from '../cli/scenario';
import { getLogger } from '../cli/utils/get_common_services';
import { RunOptions } from '../cli/utils/parse_run_cli_flags';
import { ApmFields } from '../lib/apm/apm_fields';

const scenario: Scenario<ApmFields> = async (runOptions: RunOptions) => {
Expand Down
6 changes: 3 additions & 3 deletions packages/elastic-apm-synthtrace/src/scenarios/low_throughput.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ import { random } from 'lodash';
import { apm, timerange } from '..';
import { ApmFields } from '../lib/apm/apm_fields';
import { Instance } from '../lib/apm/instance';
import { Scenario } from '../scripts/scenario';
import { getLogger } from '../scripts/utils/get_common_services';
import { RunOptions } from '../scripts/utils/parse_run_cli_flags';
import { Scenario } from '../cli/scenario';
import { getLogger } from '../cli/utils/get_common_services';
import { RunOptions } from '../cli/utils/parse_run_cli_flags';

const ENVIRONMENT = __filename;

Expand Down
6 changes: 3 additions & 3 deletions packages/elastic-apm-synthtrace/src/scenarios/many_services.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
import { random } from 'lodash';
import { apm, timerange } from '..';
import { Instance } from '../lib/apm/instance';
import { Scenario } from '../scripts/scenario';
import { getLogger } from '../scripts/utils/get_common_services';
import { RunOptions } from '../scripts/utils/parse_run_cli_flags';
import { Scenario } from '../cli/scenario';
import { getLogger } from '../cli/utils/get_common_services';
import { RunOptions } from '../cli/utils/parse_run_cli_flags';
import { ApmFields } from '../lib/apm/apm_fields';

const ENVIRONMENT = __filename;
Expand Down
6 changes: 3 additions & 3 deletions packages/elastic-apm-synthtrace/src/scenarios/monitoring.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
// Run with: node ./src/scripts/run ./src/scripts/examples/03_monitoring.ts --target=http://elastic:changeme@localhost:9200

import { stackMonitoring, timerange } from '..';
import { Scenario } from '../scripts/scenario';
import { getLogger } from '../scripts/utils/get_common_services';
import { RunOptions } from '../scripts/utils/parse_run_cli_flags';
import { Scenario } from '../cli/scenario';
import { getLogger } from '../cli/utils/get_common_services';
import { RunOptions } from '../cli/utils/parse_run_cli_flags';
import { StackMonitoringFields } from '../lib/stack_monitoring/stack_monitoring_fields';

const scenario: Scenario<StackMonitoringFields> = async (runOptions: RunOptions) => {
Expand Down
6 changes: 3 additions & 3 deletions packages/elastic-apm-synthtrace/src/scenarios/simple_trace.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
import { apm, timerange } from '..';
import { ApmFields } from '../lib/apm/apm_fields';
import { Instance } from '../lib/apm/instance';
import { Scenario } from '../scripts/scenario';
import { getLogger } from '../scripts/utils/get_common_services';
import { RunOptions } from '../scripts/utils/parse_run_cli_flags';
import { Scenario } from '../cli/scenario';
import { getLogger } from '../cli/utils/get_common_services';
import { RunOptions } from '../cli/utils/parse_run_cli_flags';

const ENVIRONMENT = __filename;

Expand Down
2 changes: 1 addition & 1 deletion packages/elastic-apm-synthtrace/src/scenarios/span_links.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import { compact, shuffle } from 'lodash';
import { apm, ApmFields, EntityArrayIterable, timerange } from '..';
import { generateLongId, generateShortId } from '../lib/utils/generate_id';
import { Scenario } from '../scripts/scenario';
import { Scenario } from '../cli/scenario';

const ENVIRONMENT = __filename;

Expand Down
1 change: 1 addition & 0 deletions packages/home/sample_data_card/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,6 @@ export {
getStoryArgTypes as getSampleDataCardStoryArgTypes,
getStoryServices as getSampleDataCardStoryServices,
getMockDataSet as getSampleDataCardMockDataSet,
// eslint-disable-next-line @kbn/imports/no_boundary_crossing
} from './mocks';
export type { Params as SampleDataCardStorybookParams } from './mocks';
1 change: 1 addition & 0 deletions packages/kbn-eslint-config/.eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,5 +258,6 @@ module.exports = {
'@kbn/imports/no_unresolvable_imports': 'error',
'@kbn/imports/uniform_imports': 'error',
'@kbn/imports/no_unused_imports': 'error',
'@kbn/imports/no_boundary_crossing': 'error',
},
};
1 change: 0 additions & 1 deletion packages/kbn-eslint-plugin-eslint/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ RUNTIME_DEPS = [
"@npm//dedent",
"@npm//eslint",
"@npm//eslint-module-utils",
"@npm//micromatch",
]

js_library(
Expand Down
41 changes: 0 additions & 41 deletions packages/kbn-eslint-plugin-eslint/README.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,47 +83,6 @@ Disallows the usage of `this` into class property initializers and enforce to de

Disables the usage of a trailing slash in a node module import.

## no-restricted-paths

Defines a set of import paths valid to be imported for a given group of files.

```
module.exports = {
overrides: [
{
files: ['**/*.{js,mjs,ts,tsx}'],
rules: {
'@kbn/eslint/no-restricted-paths': [
'error',
{
basePath: __dirname,
zones: [
{
target: [
'(src|x-pack)/plugins/**/(public|server)/**/*',
'examples/**/*',
'!(src|x-pack)/**/*.test.*',
'!(x-pack/)?test/**/*',
],
from: [
'(src|x-pack)/plugins/**/(public|server)/**/*',
'!(src|x-pack)/plugins/**/(public|server)/mocks/index.{js,mjs,ts}',
'!(src|x-pack)/plugins/**/(public|server)/(index|mocks).{js,mjs,ts,tsx}',
'!(src|x-pack)/plugins/**/__stories__/index.{js,mjs,ts,tsx}',
'!(src|x-pack)/plugins/**/__fixtures__/index.{js,mjs,ts,tsx}',
],
allowSameFolder: true,
errorMessage: 'Plugins may only import from top-level public and server modules.',
},
],
},
],
},
}
]
}
```

## require-license-header

Requires a given license header text on a group of files.
Expand Down
1 change: 0 additions & 1 deletion packages/kbn-eslint-plugin-eslint/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ module.exports = {
rules: {
'require-license-header': require('./rules/require_license_header'),
'disallow-license-headers': require('./rules/disallow_license_headers'),
'no-restricted-paths': require('./rules/no_restricted_paths'),
module_migration: require('./rules/module_migration'),
no_export_all: require('./rules/no_export_all'),
no_async_promise_body: require('./rules/no_async_promise_body'),
Expand Down
Loading