RFC: Sort Import Order (Prettier)

KevinWu098 commented 1 month ago

Description

These days, it's a super big DX thing for me to have imports be consistently grouped and ordered for easier parsing when working in (and across) file contexts.

My personal preference is to use https://github.com/IanVS/prettier-plugin-sort-imports, which builds upon https://github.com/trivago/prettier-plugin-sort-imports for a pretty quick plug-and-play experience in managing imports.

Example:

Config:

//@ts-check

/**
 * @type {import('prettier').Config}
 */
const config = {
    plugins: ['@ianvs/prettier-plugin-sort-imports'],
    importOrder: [
        '^(react/(.*)$)|^(react$)',
        '',
        '^(@mui/(.*)$)|^(@material-ui/(.*)$)',
        '',
        '<THIRD_PARTY_MODULES>',
        '',
        '^[./]',
    ],
    importOrderParserPlugins: ['typescript', 'jsx', 'decorators-legacy'],
};

module.exports = config;

Before:

import {
    Box,
    Button,
    Dialog,
    DialogActions,
    DialogContent,
    DialogContentText,
    DialogTitle,
    FormControl,
    FormControlLabel,
    Radio,
    RadioGroup,
    TextField,
    Tooltip,
} from '@material-ui/core';
import InputLabel from '@material-ui/core/InputLabel';
import { PostAdd } from '@material-ui/icons';
import { ChangeEvent, useCallback, useEffect, useState } from 'react';

import TermSelector from '../RightPane/CoursePane/SearchForm/TermSelector';
import RightPaneStore from '../RightPane/RightPaneStore';
import { addCustomEvent, openSnackbar } from '$actions/AppStoreActions';
import analyticsEnum, { logAnalytics } from '$lib/analytics';
import { warnMultipleTerms } from '$lib/helpers';
import AppStore from '$stores/AppStore';
import WebSOC from '$lib/websoc';
import { CourseInfo } from '$lib/course_data.types';
import { addCourse } from '$actions/AppStoreActions';
import { ZotCourseResponse, queryZotCourse } from '$lib/zotcourse';
import { useThemeStore } from '$stores/SettingsStore';

After:

import { ChangeEvent, useCallback, useEffect, useState } from 'react';

import {
    Box,
    Button,
    Dialog,
    DialogActions,
    DialogContent,
    DialogContentText,
    DialogTitle,
    FormControl,
    FormControlLabel,
    Radio,
    RadioGroup,
    TextField,
    Tooltip,
} from '@material-ui/core';
import InputLabel from '@material-ui/core/InputLabel';
import { PostAdd } from '@material-ui/icons';

import { addCourse, addCustomEvent, openSnackbar } from '$actions/AppStoreActions';
import analyticsEnum, { logAnalytics } from '$lib/analytics';
import { CourseInfo } from '$lib/course_data.types';
import { warnMultipleTerms } from '$lib/helpers';
import WebSOC from '$lib/websoc';
import { queryZotCourse, ZotCourseResponse } from '$lib/zotcourse';
import AppStore from '$stores/AppStore';
import { useThemeStore } from '$stores/SettingsStore';

import TermSelector from '../RightPane/CoursePane/SearchForm/TermSelector';
import RightPaneStore from '../RightPane/RightPaneStore';

This is a pretty basic example (which we can further configure for our directory structure), but I think this is a solid win for a low effort addition!

ap0nia commented 4 weeks ago

Oh actually, we do have sorting; but it's done via an ESLint plugin -- eslint-plugin-import.

An alternative plugin is eslint-plugin-simple-import.

We've configured it to throw linting errors if the imports are out of order:

❯ pnpm eslint src/components/Header/Import.tsx

/home/aponia/Projects/icssc/antalmanac/apps/antalmanac/src/components/Header/Import.tsx
  21:1   error    There should be at least one empty line between import groups                                                    import/order
  22:46  warning  '/home/aponia/Projects/icssc/antalmanac/apps/antalmanac/src/actions/AppStoreActions.ts' imported multiple times  import/no-duplicates
  26:1   error    `$lib/websoc` import should occur before import of `$stores/AppStore`                                            import/order
  26:8   warning  Using exported name 'WebSOC' as identifier for default export                                                    import/no-named-as-default
  27:1   error    `$lib/course_data.types` import should occur before import of `$lib/helpers`                                     import/order
  28:1   error    `$actions/AppStoreActions` import should occur before import of `$lib/analytics`                                 import/order
  28:27  warning  '/home/aponia/Projects/icssc/antalmanac/apps/antalmanac/src/actions/AppStoreActions.ts' imported multiple times  import/no-duplicates
  29:1   error    `$lib/zotcourse` import should occur before import of `$stores/AppStore`                                         import/order

✖ 8 problems (5 errors, 3 warnings)
  5 errors and 1 warning potentially fixable with the `--fix` option.

If you run pnpm lint --fix, then it will re-format the file to follow the configured rules.

Aside from being biased, I think it sorta makes more sense for the linter to make semantic decisions about the code since formatters are generally pretty dumb.

I do think the default order from the prettier plugin seems to make a lot more sense to me than the eslint plugin's for some reason lol. (eslint-plugin thinks that $stores should go before ../relative/path 🤷 ). This can be configured via the order setting, but I'm not sure if it's worth the effort to make a custom config 🤔

eslint-plugin-import suggested import order

import {
    Box,
    Button,
    Dialog,
    DialogActions,
    DialogContent,
    DialogContentText,
    DialogTitle,
    FormControl,
    FormControlLabel,
    Radio,
    RadioGroup,
    TextField,
    Tooltip,
} from '@material-ui/core';
import InputLabel from '@material-ui/core/InputLabel';
import { PostAdd } from '@material-ui/icons';
import { ChangeEvent, useCallback, useEffect, useState } from 'react';

import TermSelector from '../RightPane/CoursePane/SearchForm/TermSelector';
import RightPaneStore from '../RightPane/RightPaneStore';

import { addCustomEvent, openSnackbar , addCourse } from '$actions/AppStoreActions';
import analyticsEnum, { logAnalytics } from '$lib/analytics';
import { CourseInfo } from '$lib/course_data.types';
import { warnMultipleTerms } from '$lib/helpers';
import WebSOC from '$lib/websoc';
import { ZotCourseResponse, queryZotCourse } from '$lib/zotcourse';
import AppStore from '$stores/AppStore';
import { useThemeStore } from '$stores/SettingsStore';

KevinWu098 commented 4 weeks ago

Ah that's right -- it's been so long since I've worked on the codebase that I forgot about the linter. IIRC it doesn't format on save, but if we already have it, this DX thing might be better accomplished by fussing with a custom config (you have to create a custom order for Prettier anyways)

icssc / AntAlmanac