import-js / eslint-plugin-import

ESLint plugin with rules that help validate proper imports.
MIT License
5.51k stars 1.57k forks source link

Glob Pattern Help #2900

Open skyleguy opened 11 months ago

skyleguy commented 11 months ago

I am having significant issues getting the import order to accept my glob patterns for my internal import paths. I have the following 10 scenarios for what my imports look like when coming from libraries (im using NX tooling for my monorepo)

@lob/client-glist-menu-feature @lob/client-glist-menu-data-access @lob/client-glist-menu-ui @lob/client-glist-menu-util @lob/client-glist-menu-data @lob/shared-feature-menu @lob/shared-data-access-menu @lob/shared-ui-menu @lob/shared-util-menu @lob/shared-data-menu

my import/order rules look like so:

"import/order": [
          1,
          {
            "groups": ["external", "builtin", "internal", "sibling", "parent", "index"],
            "pathGroups": [
              {
                "pattern": "**/*-feature-**",
                "group": "internal"
              },
              {
                "pattern": "**/*-data-access-*",
                "group": "internal"
              },
              {
                "pattern": "**/*-ui-*",
                "group": "internal"
              },
              {
                "pattern": "**/*-util-*",
                "group": "internal"
              },
              {
                "pattern": "**/*-data-!(access)*",
                "group": "internal"
              },
              {
                "pattern": "**/*-feature",
                "group": "internal"
              },
              {
                "pattern": "**/*-data-access",
                "group": "internal"
              },
              {
                "pattern": "**/*-ui",
                "group": "internal"
              },
              {
                "pattern": "**/*-util",
                "group": "internal"
              },
              {
                "pattern": "**/*-data",
                "group": "internal"
              }
            ],
            "newlines-between": "always",
            "alphabetize": {
              "order": "asc",
              "caseInsensitive": true
            }
          }
        ]

i have even used a Quokka scratchpad using the minimatch library to confirm that these minimatch patterns should seemingly be matching against my paths but the only thing thats actually happening in my files is the alphabetizing. For example this file's imports:

import { GlistFacadeService } from '@lob/client-glist-glists-data-access';
import { favoritedRecipeText, Recipe, unfavoritedRecipeText } from '@lob/client-glist-recipes-data';
import { RecipeFacadeService } from '@lob/client-glist-recipes-data-access';
import { ArrayUtils } from '@lob/client-shared-helpers-util';
import { ConfirmActionComponent } from '@lob/client-shared-user-actions-ui';
import { Ingredient } from '@lob/shared-ingredients-data';

I would expect to look like:

import { GlistFacadeService } from '@lob/client-glist-glists-data-access';
import { RecipeFacadeService } from '@lob/client-glist-recipes-data-access';
import { ConfirmActionComponent } from '@lob/client-shared-user-actions-ui';
import { favoritedRecipeText, Recipe, unfavoritedRecipeText } from '@lob/client-glist-recipes-data';
import { ArrayUtils } from '@lob/client-shared-helpers-util';
import { Ingredient } from '@lob/shared-ingredients-data';

I know eslint is set up correctly and running since the alphabetizing is working. Anyone have any suggestions as to why this is not working? Thank you!

Oddly enough i did previously have this set up correctly and it was working fine when i was using / instead of - in the import paths, but in an attempt to make my package.json names be valid i have changed all but the first / into -. my old (working) rules with my old import paths are below:

@lob/client/glist/menu/feature
@lob/client/glist/menu/data/access
@lob/client/glist/menu/ui
@lob/client/glist/menu/util
@lob/client/glist/menu/data
@lob/shared/feature/menu
@lob/shared/data/access/menu
@lob/shared/ui/menu
@lob/shared/util/menu
@lob/shared/data/menu

"import/order": [
          1,
          {
            "groups": ["external", "builtin", "internal", "sibling", "parent", "index"],
            "pathGroups": [
              {
                "pattern": "**/data",
                "group": "internal"
              },
              {
                "pattern": "**/data/**",
                "group": "internal"
              },
              {
                "pattern": "**/data-access",
                "group": "internal"
              },
              {
                "pattern": "**/data-access/**",
                "group": "internal"
              },
              {
                "pattern": "**/ui",
                "group": "internal"
              },
              {
                "pattern": "**/ui/**",
                "group": "internal"
              },
              {
                "pattern": "**/feature",
                "group": "internal"
              },
              {
                "pattern": "**/feature/**",
                "group": "internal"
              },
              {
                "pattern": "**/util",
                "group": "internal"
              },
              {
                "pattern": "**/util/**",
                "group": "internal"
              }
            ],
            "newlines-between": "always",
            "alphabetize": {
              "order": "asc",
              "caseInsensitive": true
            }
          }
        ]

something with the minimatch has stopped working when replacing the / with -

JounQin commented 9 months ago

cc @ljharb

minimatch is very outdated in this repository, and I just found there are other unexpected behaviors:

https://github.com/import-js/eslint-plugin-import/blob/ee5fadeffff68f2300bed7f67a310496cb969d61/tests/src/rules/no-internal-modules.js#L76-L82

I think 'app/**/**' should also match "app/a" which is true on new minimatch versions as I tested.

ljharb commented 9 months ago

Hmm, app/**/** is a nonsense glob to type, since that's the same as app/**, and i think one may want app/**/*, so I'm not sure why that's in our tests.

If we can update minimatch while keeping platform support I'd love to do it.

JounQin commented 9 months ago

If we can update minimatch while keeping platform support I'd love to do it.

https://github.com/isaacs/minimatch/blob/26d281dc585af91df47cb93844e227e0ee90b7ce/package.json#L18

Unfortunately, we can't.

cc @isaacs

JounQin commented 9 months ago

Maybe micromatch@v3 is a good to try.

https://github.com/micromatch/micromatch/blob/10aacd747e9bfe3cef5b2132b085dd8f34be1582/package.json#L33

https://github.com/micromatch/micromatch#why-use-micromatch

isaacs commented 9 months ago

If your goal is to have greater correctness in glob patterns, switching from minimatch to micromatch is not the right answer. But I won't be supporting eol node versions. The source is open, please feel free to fork and patch if you like.

JounQin commented 9 months ago

If your goal is to have greater correctness in glob patterns, switching from minimatch to micromatch is not the right answer.

Thanks @isaacs, can you help to explain in details?

levithomason commented 7 months ago

I'm also unable to get **/shared/** to work:

Given

import type { InferAttributes, WhereOptions } from "sequelize";
import { dotProduct } from "../../../shared/math.js";
import { getDB } from "../../database/index.js";

And:

"newlines-between": "always",
"distinctGroup": true,
"pathGroups": [{ "pattern": "<pattern>", "position": "after", "group": "external" }],

Separate group created for ../../../shared/math.js:

**/shared/***/shared/*.*

**/shared/****/../shared/****/../../shared/**../**/../shared/**../../**/shared/**

**shared***shared*shared

It only works if all three .. are included in the pattern:

../../../shared/****/../../../shared/**../**/../../shared/**../../**/../shared/**../../../**/shared/**

Ugly hack workaround, match 1-3 levels up:

../{,../}{,../}shared/**