[RFC]: Expand ESLint import order rules

[realStandal]

Summary

At the moment I have two key changes to suggest, happy to lop some more in (or generally to ESLint) if the community has any.

• Add automated grouping of the top-level directories in each side's src/ directory.

From my meager testing (I just updated my project to v2.1), the following will take effect as directories are added/removed.

const readDirectory = (path) =>
  readdirSync(path, { withFileTypes: true })
    .filter((d) => d.isDirectory())
    .map((d) => d.name)

const srcPaths = () => {
  //
  const rwPaths = getPaths()

  const paths = [
    ...new global.Set([
      ...readDirectory(rwPaths.api.src),
      ...readDirectory(rwPaths.web.src),
    ]),
  ].sort()

  return paths
}

module.exports = {
  ...
  rules: {
    'import/order': [
      {
        ...
        pathGroups: [
          ...srcPaths().map((path) => ({
            pattern: `src/${path}/**`,
            group: 'parent',
            position: 'after',
          })),
        ],
      },
    ],
  },
}

• Add types/** as a dedicated section, appearing after src/**.

Motivation

• When numerous src/** directories are imported within a single file, the glob of imports can become cumbersome to navigate at a glance - it being hard to locate a specific import within the combined list.
• At present, imports to types/** will be ordered alongside external imports, despite this directory being local to the application.

Detailed proposal

The pseudo-code in the "Summary" section would be best added to the shared ESLint configuration.

The types/** rule would, similarly, get added here to be shared by each side.

Are you interested in working on this?

• [X] I'm interested in working on this

[Tobbe]

One thing that's been bugging me about import/order is that whenever VSCode auto-imports something for me I get red squiggles. So it would be really nice if we could make VSCode insert auto-imported modules in the right place. Is that possible?

[Tobbe]

@jtoar @dac09 Do any of you have any input on this? You were both involved in this semi-related PR from a couple of months ago https://github.com/redwoodjs/redwood/pull/4954

Ohh, and I just now noticed that there's been some activity on that PR recently related to glob imports, so @realStandal please do have a look at those comments too

[orta]

Is that possible?

No, this comes from TypeScript - and typescript has its own simple heuristic for where to put them

[Tobbe]

Bummer! Guess I'll just have to disable the eslint rule then 🙂 I never look at my imports anyway. That's for VSCode to handle 🙂

[realStandal]

@Tobbe thanks for looping that PR in, from my quick pass it looks like the gist is @dac09 and @jtoar were against the opinionative separation of src/* directories (please correct me if I'm putting words in y'all's mouth) - I had the hunch that suggestion would get push-back.

No biggie to me if it's a hill someone's dying on, I can turn that part of the suggestion into a forum/cookbook post so it's available for those who want.

I still think the types/* separation should get pushed to the framework, though.

[Tobbe]

@jtoar @dac09 I'm assigning you two to get your input too

[jtoar]

This is on my list. @realStandal while I'm getting reviews done, could you provide a before-after example of what your changes do? (And where would those changes go if implemented? packages/eslint-config/shared.js?) I read this briefly but don't quite understand the implications yet. So I don't have any push back to the suggestion yet!

[realStandal]

@jtoar (these are taken straight from my app, so please excuse any specificity)

Before:

import { Prisma } from '@prisma/client'
import type { UserRole } from 'types/graphql' // <-- Here is the "types/**" import, located with other external imports.

import { AuthenticationError, ForbiddenError } from '@redwoodjs/graphql-server'

// The following is the "numerous src/** directories" I spoke about in the original post
import { db } from 'src/lib/db'
import Sentry from 'src/lib/sentry'
import { getBilling } from 'src/services/billing'

After:

import { Prisma } from '@prisma/client'

import { AuthenticationError, ForbiddenError } from '@redwoodjs/graphql-server'

import { db } from 'src/lib/db'
import Sentry from 'src/lib/sentry'

import { getBilling } from 'src/services/billing'

import type { UserRole } from 'types/graphql'

---

I believe the following could be used for types/**, and can be added below the current pathGroups in shared.js

{
  pattern: 'types/**',
  group: 'parent',
  position: 'after',
},

The src/** definition is more complex, but the code I provided in the original post would:

• Include an additional import to shared.js for readdirSync and getPaths
• Adding readDirectory and srcPaths as functions to shared.js
• Include mapping the results of srcPaths onto pathGroups, each result being its own group.

Here is the gist of shared.js.

Please let me know if a PR would be easier to work with (or if I should have made this suggestion as a PR to begin with - all a learning experience (for me) :) )

[jtoar]

Thanks @realStandal! That's enough information for now. We discussed this rule again last meeting (not only your suggestion, but the rule as a whole). It's very divisive.

Before you posted this, the only other feedback I've seen from the community was this thread: https://discord.com/channels/679514959968993311/988820117624336434. I'm guessing you like the rule (correct me if I'm wrong!), but do you happen to know how other Redwood developers feel? No worries if not, just trying to get a sense for things.

We didn't come to any conclusions last time we talked as a team cause we had to move on to other topics. So I'll keep bringing this one up. This is all speculative, but I'm guessing we'll at least do the following:

• make the rule easy to turn off
• make sure babel globstar imports are their own group