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

[web] Add eslint-plugin-simple-import-sort #49368

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
  •  
  •  
  •  
12 changes: 12 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion tsconfig.base.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"teleterm/*": ["web/packages/teleterm/src/*"],
"e-teleport/*": ["e/web/teleport/src/*"],
"gen-proto-js/*": ["gen/proto/js/*"],
"gen-proto-ts/*": ["gen/proto/ts/*"],
"gen-proto-ts/*": ["gen/proto/ts/*"]
}
}
}
2 changes: 1 addition & 1 deletion web/@types/styled-components.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

import { CSSProp } from 'styled-components';
import 'react';
import { CSSProp } from 'styled-components';

import { Theme } from 'design/theme/themes/types';

Expand Down
109 changes: 79 additions & 30 deletions web/packages/build/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,44 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

const path = require('path');
const tsConfigBase = require(
path.join(path.resolve(__dirname, '../../..'), 'tsconfig.base.json')
);

const appPackages = ['teleport', 'e-teleport', 'teleterm'];
const libraryPackages = Object.keys(tsConfigBase.compilerOptions.paths)
.map(p => p.split('/')[0])
.filter(p => !appPackages.includes(p) && !p.startsWith('gen-proto-'));

const excludedPackagesPattern = [
...appPackages,
...libraryPackages,
'gen-proto-js',
'gen-proto-ts',
].join('|');

// importSortGroups are passed to `simple-import-sort` plugin to sort imports. Each group is an array of regexes.
// Imports are sorted by the first group that matches them, with newlines inserted between groups.
const importSortGroups = [
// Side-effect imports (e.g. CSS)
['^\u0000(?!.*\u0000$)'],
kiosion marked this conversation as resolved.
Show resolved Hide resolved
// Node.js builtins prefixed w/ 'node:'
['^node:(?!.*\u0000$)'],
// Any 3rd-party imports (e.g. 'react', 'styled-components')
[`^(?![./])(?!(?:${excludedPackagesPattern})(?:/|$))(?!.*\u0000$).+`],
Copy link
Member

Choose a reason for hiding this comment

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

What's the deal with \u0000 in every import group? The diff that I posted, which was based on the default import groups, had it only in side effect imports.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That represents a byte present at the end of type-only imports, so in order to keep those sorted to the bottom we need a negative lookahead at the end of previous import groups

// Our library packages (e.g. 'shared', 'design')
[`^(?:${libraryPackages.join('|')})(?:/)?(?!.*\u0000$)`],
// Our app packages (e.g. '(e-)?teleport', 'teleterm')
[`^(?:${appPackages.join('|')})(?:/)?(?!.*\u0000$)`],
// Imports from 'gen-proto-ts/js'
['^gen-proto-(?:j|t)s(?:/)?(?!.*\u0000$)'],
kiosion marked this conversation as resolved.
Show resolved Hide resolved
// Absolute and relative imports
['^[^.](?!.*\u0000$)', '^\\.(?!.*\u0000$)'],
// Type imports
['\u0000$'],
];

module.exports = {
parser: '@typescript-eslint/parser',
parserOptions: {
Expand All @@ -42,7 +80,7 @@ module.exports = {
'plugin:import/warnings',
'plugin:import/typescript',
],
plugins: ['react', 'babel', 'import', 'react-hooks'],
plugins: ['react', 'babel', 'import', 'simple-import-sort', 'react-hooks'],
overrides: [
{
files: ['**/*.test.{ts,tsx,js,jsx}'],
Expand All @@ -62,56 +100,66 @@ module.exports = {
'jest/prefer-strict-equal': 0,
'jest/prefer-inline-snapshots': 0,
'jest/require-top-level-describe': 0,
'jest/no-large-snapshots': ['warn', { maxSize: 200 }],
'jest/no-large-snapshots': [1, { maxSize: 200 }],
},
},
// Allow require imports in .js files, as migrating our project to ESM modules requires a lot of
// Allow `require` imports in .js files, as migrating our project to ESM modules requires a lot of
// changes.
{
files: ['**/*.js'],
rules: {
'@typescript-eslint/no-require-imports': 'warn',
'@typescript-eslint/no-require-imports': 1,
},
},
// Allow `require` imports & no import sorting in this file
{
files: ['**/.eslintrc.js'],
rules: {
'@typescript-eslint/no-require-imports': 0,
'simple-import-sort/imports': 0,
'import/newline-after-import': 0,
},
},
],
// Severity should be one of the following:
// "off" or 0 - turn the rule off
// "warn" or 1 - turn the rule on as a warning (doesn’t affect exit code)
// "error" or 2 - turn the rule on as an error (exit code is 1 when triggered)
rules: {
'import/order': [
'error',
{
groups: [
'builtin',
'external',
'internal',
'parent',
'sibling',
'index',
'object',
'type',
],
'newlines-between': 'always-and-inside-groups',
},
],
// Sort imports/exports
'import/order': 0,
'simple-import-sort/imports': [2, { groups: importSortGroups }],
'simple-import-sort/exports': 1,
// Make sure imports are first
'import/first': 2,
// Add newline after all imports
'import/newline-after-import': 2,
// Merge duplicate imports (duplicate type-only imports are allowed)
'import/no-duplicates': 2,

// typescript-eslint recommends to turn import/no-unresolved off.
// https://typescript-eslint.io/troubleshooting/typed-linting/performance/#eslint-plugin-import
'import/no-unresolved': 0,
'no-unused-vars': 'off', // disabled to allow the typescript one to take over and avoid errors in reporting
'@typescript-eslint/no-unused-vars': ['error'],
'no-unused-expressions': 'off',
'no-unused-vars': 0, // disabled to allow the typescript one to take over and avoid errors in reporting
'@typescript-eslint/no-unused-vars': [
2,
// Allow unused vars/args that start with an underscore
{
varsIgnorePattern: '^_',
argsIgnorePattern: '^_',
},
Comment on lines +146 to +150
Copy link
Member

Choose a reason for hiding this comment

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

👍, Bartosz was complaining about this recently.

],
'no-unused-expressions': 0,
'@typescript-eslint/no-unused-expressions': [
'error',
2,
{ allowShortCircuit: true, allowTernary: true, enforceForJSX: true },
],
'@typescript-eslint/no-empty-object-type': [
'error',
2,
// with-single-extends is needed to allow for interface extends like we have in jest.d.ts.
{ allowInterfaces: 'with-single-extends' },
],

// Severity should be one of the following:
// "off" or 0 - turn the rule off
// "warn" or 1 - turn the rule on as a warning (doesn’t affect exit code)
// "error" or 2 - turn the rule on as an error (exit code is 1 when triggered)

// <TODO> Enable these rules after fixing all existing issues
'@typescript-eslint/no-use-before-define': 0,
'@typescript-eslint/indent': 0,
Expand All @@ -125,6 +173,7 @@ module.exports = {
'@typescript-eslint/prefer-interface': 0,
'@typescript-eslint/no-empty-function': 0,
'@typescript-eslint/no-this-alias': 0,
'@typescript-eslint/no-unnecessary-type-assertion': 0,

// </TODO>
'comma-dangle': 0,
Expand Down
3 changes: 1 addition & 2 deletions web/packages/build/jest/jest-environment-patched-jsdom.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { TextEncoder, TextDecoder } from 'node:util';

import { TestEnvironment as JSDOMEnvironment } from 'jest-environment-jsdom';
import { TextDecoder, TextEncoder } from 'node:util';

// When using jest-environment-jsdom, TextEncoder and TextDecoder are not defined. This poses a
// problem when writing tests for code which uses TextEncoder and TextDecoder directly or that
Expand Down
4 changes: 1 addition & 3 deletions web/packages/build/jest/setupTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,10 @@
*/

import 'whatwg-fetch';

import failOnConsole from 'jest-fail-on-console';
import crypto from 'node:crypto';
import path from 'node:path';

import failOnConsole from 'jest-fail-on-console';

let entFailOnConsoleIgnoreList = [];
try {
// Cannot do `await import` yet here.
Expand Down
1 change: 1 addition & 0 deletions web/packages/build/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
"eslint-plugin-jest-dom": "^5.4.0",
"eslint-plugin-react": "^7.37.0",
"eslint-plugin-react-hooks": "^5.0.0",
"eslint-plugin-simple-import-sort": "^12.1.1",
"eslint-plugin-testing-library": "^6.3.0",
"jest-environment-jsdom": "^29.7.0",
"jest-fail-on-console": "^3.3.0",
Expand Down
5 changes: 2 additions & 3 deletions web/packages/build/vite/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,12 @@

import { existsSync, readFileSync } from 'fs';
import { resolve } from 'path';

import { defineConfig } from 'vite';
import { visualizer } from 'rollup-plugin-visualizer';
import { defineConfig } from 'vite';
import wasm from 'vite-plugin-wasm';

import { htmlPlugin, transformPlugin } from './html';
import { generateAppHashFile } from './apphash';
import { htmlPlugin, transformPlugin } from './html';
import { reactPlugin } from './react.mjs';
import { tsconfigPathsPlugin } from './tsconfigPaths.mjs';

Expand Down
5 changes: 2 additions & 3 deletions web/packages/build/vite/html.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,11 @@

import { readFileSync } from 'fs';
import { get } from 'https';
import { resolve } from 'path';

import { JSDOM } from 'jsdom';
import { resolve } from 'path';

import type { Plugin } from 'vite';
import type { IncomingHttpHeaders } from 'http';
import type { Plugin } from 'vite';

function getHTML(target: string, headers: IncomingHttpHeaders) {
return new Promise<{ data: string; headers: IncomingHttpHeaders }>(
Expand Down
1 change: 0 additions & 1 deletion web/packages/design/src/Alert/Alert.story.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import React from 'react';
import { Restore } from 'design/Icon';

import { Box } from '..';

import { Alert, AlertProps, Banner } from './Alert';

export default {
Expand Down
14 changes: 7 additions & 7 deletions web/packages/design/src/Alert/Alert.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,18 @@

import React, { useState } from 'react';
import styled, { useTheme } from 'styled-components';
import { style, color, ColorProps } from 'styled-system';
import { color, ColorProps, style } from 'styled-system';

import { IconProps } from 'design/Icon/Icon';

import { space, SpaceProps, width, WidthProps } from '../system';
import { Theme } from '../theme';
import * as Icon from '../Icon';
import Flex from '../Flex';
import Text from '../Text';
import Box from '../Box';
import { ButtonFill, ButtonIntent, Button } from '../Button';
import { Button, ButtonFill, ButtonIntent } from '../Button';
import ButtonIcon from '../ButtonIcon';
import Flex from '../Flex';
import * as Icon from '../Icon';
import { space, SpaceProps, width, WidthProps } from '../system';
import Text from '../Text';
import { Theme } from '../theme';

const linkColor = style({
prop: 'linkColor',
Expand Down
1 change: 0 additions & 1 deletion web/packages/design/src/Box/Box.story.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
*/

import React from 'react';

import styled from 'styled-components';

import Box from './Box';
Expand Down
15 changes: 7 additions & 8 deletions web/packages/design/src/Button/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,20 @@
import React from 'react';
import styled, { CSSObject } from 'styled-components';

import { shouldForwardProp as defaultValidatorFn } from 'design/ThemeProvider';

import {
space,
width,
height,
alignSelf,
AlignSelfProps,
gap,
GapProps,
height,
HeightProps,
space,
SpaceProps,
width,
WidthProps,
HeightProps,
AlignSelfProps,
GapProps,
} from 'design/system';
import { Theme } from 'design/theme/themes/types';
import { shouldForwardProp as defaultValidatorFn } from 'design/ThemeProvider';

export type ButtonProps<E extends React.ElementType> =
React.ComponentPropsWithoutRef<E> &
Expand Down
17 changes: 7 additions & 10 deletions web/packages/design/src/Button/buttons.story.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,27 +17,24 @@
*/

import React from 'react';

import styled from 'styled-components';

import { Input, MenuItem } from 'design';

import ButtonLink from '../ButtonLink';
import ButtonIcon from '../ButtonIcon';
import * as icons from '../Icon';
import Flex from '../Flex';

import ButtonLink from '../ButtonLink';
import { ButtonWithMenu } from '../ButtonWithMenu';

import Flex from '../Flex';
import * as icons from '../Icon';
import {
Button,
ButtonBorder,
ButtonFill,
ButtonPrimary,
ButtonProps,
ButtonSecondary,
ButtonWarning,
ButtonBorder,
ButtonText,
ButtonProps,
ButtonFill,
ButtonWarning,
} from '.';

export default {
Expand Down
2 changes: 1 addition & 1 deletion web/packages/design/src/ButtonIcon/ButtonIcon.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import React from 'react';
import styled from 'styled-components';

import { space, color, alignSelf } from 'design/system';
import { alignSelf, color, space } from 'design/system';

const sizeMap = {
0: {
Expand Down
1 change: 1 addition & 0 deletions web/packages/design/src/ButtonIcon/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,5 @@
*/

import ButtonIcon from './ButtonIcon';

export default ButtonIcon;
1 change: 1 addition & 0 deletions web/packages/design/src/ButtonLink/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,5 @@
*/

import ButtonLink from './ButtonLink';

export default ButtonLink;
Loading
Loading