-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Enterprise Search] Add eslint import/order rules #90530
Changes from all commits
03de116
9830d7d
fab9cbe
f5d08c4
7955ab1
9e22ec0
b4d6796
124aee9
1a06f1e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1119,6 +1119,39 @@ module.exports = { | |
// All files | ||
files: ['x-pack/plugins/enterprise_search/**/*.{ts,tsx}'], | ||
rules: { | ||
'import/order': [ | ||
'error', | ||
{ | ||
groups: ['unknown', ['builtin', 'external'], 'internal', 'parent', 'sibling', 'index'], | ||
pathGroups: [ | ||
{ | ||
pattern: | ||
'{../../../../../../,../../../../../,../../../../,../../../,../../,../}{common/,*}__mocks__{*,/**}', | ||
group: 'unknown', | ||
}, | ||
{ | ||
pattern: '{**,.}/*.mock', | ||
group: 'unknown', | ||
}, | ||
{ | ||
pattern: 'react*', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I opted to group React-adjacent imports (e.g. I don't feel super strongly if people would rather have only the main |
||
group: 'external', | ||
position: 'before', | ||
}, | ||
{ | ||
pattern: '{@elastic/**,@kbn/**,src/**}', | ||
group: 'internal', | ||
}, | ||
], | ||
pathGroupsExcludedImportTypes: [], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is required for the React external rule to work. See import-js/eslint-plugin-import#1874 |
||
alphabetize: { | ||
order: 'asc', | ||
caseInsensitive: true, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I listed I left it in anyway as I think we would still probably want this rule if we were on the old ent-search repo with PascalCased component filenames, but would be curious to hear what others think and if my assumption was wrong there |
||
}, | ||
'newlines-between': 'always-and-inside-groups', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Docs link) I opted for import { SharedHelperA } from '../../../shared/a'
import { SharedHelperB } from '../../../shared/b'
import { SharedHelperC } from '../../../shared/c'
import { ProductSpecificHelperA } from '../app_search/utils/util_a';
import { ProductSpecificHelperB } from '../app_search/utils/util_b'; Actual example of newlines within a group being used to make code more readable in our import { registerTelemetryUsageCollector as registerASTelemetryUsageCollector } from './collectors/app_search/telemetry';
import { registerTelemetryUsageCollector as registerESTelemetryUsageCollector } from './collectors/enterprise_search/telemetry';
import { registerTelemetryUsageCollector as registerWSTelemetryUsageCollector } from './collectors/workplace_search/telemetry';
import { checkAccess } from './lib/check_access';
import {
EnterpriseSearchRequestHandler,
IEnterpriseSearchRequestHandler,
} from './lib/enterprise_search_request_handler';
import { registerAppSearchRoutes } from './routes/app_search';
import { registerConfigDataRoute } from './routes/enterprise_search/config_data';
import { registerTelemetryRoute } from './routes/enterprise_search/telemetry';
import { registerWorkplaceSearchRoutes } from './routes/workplace_search';
import { appSearchTelemetryType } from './saved_objects/app_search/telemetry';
import { enterpriseSearchTelemetryType } from './saved_objects/enterprise_search/telemetry';
import { workplaceSearchTelemetryType } from './saved_objects/workplace_search/telemetry'; |
||
}, | ||
], | ||
'import/newline-after-import': 'error', | ||
'react-hooks/exhaustive-deps': 'off', | ||
'react/jsx-boolean-value': ['error', 'never'], | ||
}, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes this is absolutely horrifying. No there is no way else to do this. You would think that
**/__mocks__/**
would work. It absolutely does not for whatever reason (..
doesn't count as**
).See: import-js/eslint-plugin-import#1632
So yeah, I guess let's hope we don't go 7 folders in deep from a
__mocks__
file, or we're adding yet another another,
of 7x../
s :dead_inside:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Y'all are also free to test this yourselves (glob tester links are in the PR description, under resources) but I can also reassure you I spent several hours of my life yesterday and today trying to get this pathGroup work and this was the only way it did.