Skip to content

Commit 435f7e1

Browse files
authored
ref: replace eslint-plugin-simple-import-sort with prettier-plugin-sort-imports (#97578)
Why? - Formatting is a concern for the formatter, not the linter - We always lint all files in CI, which can be slow, but we only format what we touch --> The more we move to the formatter, the faster things get. This should save around 2% of lint execution time. Not too much but still, this was one of the slower rules and I could turn off two additional rules that are now covered by prettier. I tried to keep the same ordering rules we have, but still, 559 files are formatted differently now. There's two reasons for this: 1. We have a setting that says [nodeJs built-ins should come first](https://github.com/getsentry/sentry/blob/6d867a00a2ba346378409c5cbe87a776d6e73854/eslint.config.mjs#L616-L617), but it seems that it doesn't work. I've kept this for prettier and now, node imports are actually moved to the top, leading to some diffs. 2. The prettier plugin always sorts type-imports lasts, and this isn't really configurable. I think this is a good change, but leads to most diffs looking something like this: ``` - import {CheckoutType, type Plan, ReservedBudgetCategoryType} from 'getsentry/types'; + import {CheckoutType, ReservedBudgetCategoryType, type Plan} from 'getsentry/types'; ``` I think the current order was purely alphabetical, disregarding type modifiers. Oh and one more thing: The plugin never re-orders side-effect imports, so they stay where they are.
1 parent 36bdfbc commit 435f7e1

File tree

565 files changed

+849
-886
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

565 files changed

+849
-886
lines changed

api-docs/index.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
import yaml from 'js-yaml';
2-
import JsonRefs from 'json-refs';
31
import fs from 'node:fs';
42
import path from 'node:path';
53

4+
import yaml from 'js-yaml';
5+
import JsonRefs from 'json-refs';
6+
67
function dictToString(dict) {
78
const res: string[] = [];
89
for (const [k, v] of Object.entries(dict)) {

api-docs/openapi-diff.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
import yaml from 'js-yaml';
2-
import jsonDiff from 'json-diff';
31
import fs from 'node:fs';
42
import https from 'node:https';
53

4+
import yaml from 'js-yaml';
5+
import jsonDiff from 'json-diff';
6+
67
async function main() {
78
const openApiData = await new Promise((resolve, reject) =>
89
https.get(

api-docs/watch.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {spawn} from 'node:child_process';
22
import {join} from 'node:path';
33
import {stderr, stdout} from 'node:process';
4+
45
import sane from 'sane';
56

67
const watcherPy = sane(join(__dirname, '../src/sentry'));

build-utils/last-built-plugin.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
import type {Compiler, RspackPluginInstance} from '@rspack/core';
21
import fs from 'node:fs';
32
import path from 'node:path';
43

4+
import type {Compiler, RspackPluginInstance} from '@rspack/core';
5+
56
type Options = {
67
basePath: string;
78
};

build-utils/peggy-loader.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import peggy from 'peggy';
21
import type {LoaderDefinitionFunction} from '@rspack/core';
2+
import peggy from 'peggy';
33

44
const peggyLoader: LoaderDefinitionFunction = source => {
55
// https://peggyjs.org/documentation.html#generating-a-parser-javascript-api

build-utils/remark-unwrap-mdx-paragraphs.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
import type {Node, Literal, Parent} from 'hast';
2-
1+
import type {Literal, Node, Parent} from 'hast';
32
import {visit} from 'unist-util-visit';
43

54
/**

build-utils/ts-extract-gettext.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
import fs from 'node:fs/promises';
2-
import path from 'node:path';
2+
import path, {resolve} from 'node:path'; // Added for module check
33
import {fileURLToPath} from 'node:url';
4-
import {resolve} from 'node:path'; // Added for module check
54

6-
import {glob} from 'tinyglobby';
7-
import ts from 'typescript';
85
import {po} from 'gettext-parser';
96
import type {GetTextTranslation, GetTextTranslations} from 'gettext-parser';
7+
import {glob} from 'tinyglobby';
8+
import ts from 'typescript';
109

1110
const FUNCTION_NAMES: Record<string, string[]> = {
1211
gettext: ['msgid'],

config/build-chartcuterie.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
/* eslint-disable import/no-nodejs-modules, no-console */
22

3-
import * as esbuild from 'esbuild';
43
import childProcess from 'node:child_process';
54
import fs from 'node:fs/promises';
65
import path from 'node:path';
76
import {fileURLToPath} from 'node:url';
87

8+
import * as esbuild from 'esbuild';
9+
910
const scriptDir = path.dirname(fileURLToPath(import.meta.url));
1011
const workspaceRoot = path.resolve(scriptDir, '..');
1112
const packageJsonPath = path.join(workspaceRoot, 'package.json');

eslint.config.mjs

Lines changed: 9 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
import * as emotion from '@emotion/eslint-plugin';
1313
import eslint from '@eslint/js';
1414
import pluginQuery from '@tanstack/eslint-plugin-query';
15-
import {globalIgnores} from 'eslint/config';
1615
import prettier from 'eslint-config-prettier';
1716
// @ts-expect-error TS(7016): Could not find a declaration file
1817
import boundaries from 'eslint-plugin-boundaries';
@@ -25,14 +24,13 @@ import react from 'eslint-plugin-react';
2524
import reactHooks from 'eslint-plugin-react-hooks';
2625
// @ts-expect-error TS(7016): Could not find a declaration file
2726
import sentry from 'eslint-plugin-sentry';
28-
import simpleImportSort from 'eslint-plugin-simple-import-sort';
2927
import testingLibrary from 'eslint-plugin-testing-library';
3028
// @ts-expect-error TS (7016): Could not find a declaration file
3129
import typescriptSortKeys from 'eslint-plugin-typescript-sort-keys';
3230
import unicorn from 'eslint-plugin-unicorn';
31+
import {globalIgnores} from 'eslint/config';
3332
import globals from 'globals';
3433
import invariant from 'invariant';
35-
import {builtinModules} from 'node:module';
3634
import typescript from 'typescript-eslint';
3735

3836
invariant(react.configs.flat, 'For typescript');
@@ -386,7 +384,6 @@ export default typescript.config([
386384
rules: {
387385
// https://github.com/import-js/eslint-plugin-import/blob/main/config/recommended.js
388386
...importPlugin.flatConfigs.recommended.rules,
389-
'import/newline-after-import': 'error', // https://prettier.io/docs/en/rationale.html#empty-lines
390387
'import/no-absolute-path': 'error',
391388
'import/no-amd': 'error',
392389
'import/no-anonymous-default-export': 'error',
@@ -600,52 +597,6 @@ export default typescript.config([
600597
],
601598
},
602599
},
603-
{
604-
name: 'plugin/simple-import-sort',
605-
// https://github.com/lydell/eslint-plugin-simple-import-sort
606-
plugins: {'simple-import-sort': simpleImportSort},
607-
rules: {
608-
'import/order': 'off',
609-
'sort-imports': 'off',
610-
'simple-import-sort/imports': [
611-
'error',
612-
{
613-
groups: [
614-
// Side effect imports.
615-
[String.raw`^\u0000`],
616-
617-
// Node.js builtins.
618-
[`^(${builtinModules.join('|')})(/|$)`],
619-
620-
// Packages. `react` related packages come first.
621-
['^react', String.raw`^@?\w`],
622-
623-
// Test should be separate from the app
624-
['^(sentry-test|getsentry-test)(/.*|$)'],
625-
626-
// Internal packages.
627-
['^(sentry-locale|sentry-images)(/.*|$)'],
628-
629-
['^ui(/.*|$)'],
630-
631-
['^(app|sentry)(/.*|$)'],
632-
633-
// Getsentry packages.
634-
['^(admin|getsentry)(/.*|$)'],
635-
636-
// Style imports.
637-
[String.raw`^.+\.less$`],
638-
639-
// Parent imports. Put `..` last.
640-
[String.raw`^\.\.(?!/?$)`, String.raw`^\.\./?$`],
641-
642-
// Other relative imports. Put same-folder imports and `.` last.
643-
[String.raw`^\./(?=.*/)(?!/?$)`, String.raw`^\.(?!/?$)`, String.raw`^\./?$`],
644-
],
645-
},
646-
],
647-
},
648-
},
649600
{
650601
name: 'plugin/sentry',
651602
// https://github.com/getsentry/eslint-config-sentry/tree/master/packages/eslint-plugin-sentry/docs/rules
@@ -755,6 +706,14 @@ export default typescript.config([
755706
{
756707
name: 'plugin/prettier',
757708
...prettier,
709+
rules: {
710+
// import sorting is handled with prettier-plugin-sort-imports
711+
'import/order': 'off',
712+
'sort-imports': 'off',
713+
'import/newline-after-import': 'off',
714+
// prettier-plugin-sort-imports always combines imports
715+
'import/no-duplicates': 'off',
716+
},
758717
},
759718
{
760719
name: 'files/*.config.*',

jest.config.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
import type {Config} from '@jest/types';
1+
import {execFileSync} from 'node:child_process';
22
import path from 'node:path';
33
import process from 'node:process';
4-
import {execFileSync} from 'node:child_process';
4+
55
import type {TransformOptions} from '@babel/core';
6+
import type {Config} from '@jest/types';
67

78
const babelConfig: TransformOptions = {
89
presets: [

0 commit comments

Comments
 (0)