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

Import/order with .svg file patterns #1858

Closed
nirtamir2 opened this issue Jul 19, 2020 · 17 comments
Closed

Import/order with .svg file patterns #1858

nirtamir2 opened this issue Jul 19, 2020 · 17 comments
Labels

Comments

@nirtamir2
Copy link

nirtamir2 commented Jul 19, 2020

Hi,
Thank you for creating this amazing ESLint plugin.

I've configured that any files matches @hadeshe/** will be after the external group, and it works well.
I want to order my imports so that they will have a group of files matching .svg in the end.

Here is my config:

{
  "import/order": [
    "warn",
    {
      "groups": [
        ["builtin", "external"],
        "internal",
        ["parent", "index", "sibling"]
      ],
      "pathGroups": [
        // Works well
        {
          "pattern": "@hadeshe/**",
          "group": "external",
          "position": "after"
        },
        // Does not work
        {
          "pattern": "**/*.svg",
          "group": "object",
          "position": "after"
        }
      ],
      "pathGroupsExcludedImportTypes": ["builtin"],
      "newlines-between": "always",
      "alphabetize": {
        "order": "asc"
      }
    }
  ]
}

Expected:

import React from "react";

import { colors } from "@hadeshe/ui-core";

import { Routes } from "../Routes";
import { IRootStackNavigationProps } from "../types";
import TrophyIcon from "./assets/trophy.svg";
import { LeagueTab } from "./match/LeagueTab";
import { PlayerTab } from "./player/PlayerTab";
import PlayersIconOutline from "./assets/soccer-jersey-outline.svg";
import PlayersIcon from "./assets/soccer-jersey.svg";
import TrophyIconOutline from "./assets/trophy-outline.svg";

Actual:

import React from "react";

import { colors } from "@hadeshe/ui-core";

import { Routes } from "../Routes";
import { IRootStackNavigationProps } from "../types";
import PlayersIconOutline from "./assets/soccer-jersey-outline.svg";
import PlayersIcon from "./assets/soccer-jersey.svg";
import TrophyIconOutline from "./assets/trophy-outline.svg";
import TrophyIcon from "./assets/trophy.svg";
import { LeagueTab } from "./match/LeagueTab";
import { PlayerTab } from "./player/PlayerTab";

How to order .svg imports to be the last group?

@nirtamir2 nirtamir2 changed the title question: Import/order with svg file patterns Import/order with .svg file patterns Jul 19, 2020
@ljharb
Copy link
Member

ljharb commented Jul 20, 2020

The "object" group isn't included by default or in your config; you have to explicitly add it to groups if you want it applied.

@nirtamir2
Copy link
Author

nirtamir2 commented Jul 20, 2020

Hi @laysent
Thank you for your help.
I figured out what was the problem.
It's more related to the minimatch library.
As in this minimatch issue and this issue issues, my pattern for all SVG files is not correct.
I did not find a way to match all the paths that end with .svg and can include dots for relative paths. So meanwhile I configured a temporary solution

{
rules: {
"import/order": [
      "warn",
      {
        groups: [
          ["builtin", "external"],
          "internal",
          ["parent", "index", "sibling"],
        ],
        pathGroups: [
          {
            pattern: "./assets/*.svg",
            group: "sibling",
            position: "after",
          },
          {
            pattern: "../assets/*.svg",
            group: "sibling",
            position: "after",
          },
        ],
        pathGroupsExcludedImportTypes: ["builtin"],
        "newlines-between": "always",
        alphabetize: {
          order: "asc",
        },
      },
    ]}
  settings: {
    // Refer @hadeshe packages as internal for import/order rule
    "import/internal-regex": "^@hadeshe/",
  },
}

I also notice #1632 issue (using regex for pattern matching ) and it can help in my case.

@ljharb
Copy link
Member

ljharb commented Oct 3, 2020

@nirtamir2 does this mean this issue is resolved, and #1632 handles any further issues?

@nirtamir2
Copy link
Author

Not yet.
I want to create a group of SVG as the last import statement.
Regex could potentially solve it with /\.svg$/ pattern.
I want a glob pattern that matches all .svg files.
I tried *.svg **.svg **/*.svg and it did not works well.
Can this pattern be achieved with a glob?

@ljharb
Copy link
Member

ljharb commented Oct 3, 2020

I don't understand why *.svg etc wouldn't work. Have you specified svg in the import plugin's extensions settings?

@nirtamir2
Copy link
Author

nirtamir2 commented Oct 3, 2020

I tried with this .eslintrc.js:

  "import/order": [
      "warn",
      {
        groups: [
          ["builtin", "external"],
          "internal",
          ["parent", "index", "sibling"],
        ],
        pathGroups: [
          {
            pattern: "*.svg",
            group: "sibling",
            position: "after",
          },
        ],
        pathGroupsExcludedImportTypes: ["builtin"],
        "newlines-between": "always",
        alphabetize: {
          order: "asc",
        },
      },
    ],
  },
  settings: {
    // Refer @hadeshe packages as internal for import/order rule
    "import/internal-regex": "^@hadeshe/",
    "import/extensions": [".js", ".jsx", ".ts", ".tsx", ".svg", ".json"],
  },

And it is still not working. I get the same result as before.
Is there a way that I can help more with this issue?

@ljharb
Copy link
Member

ljharb commented Oct 3, 2020

I think you'd also need a resolver that can understand .svg - node can't, so presumably you're using webpack, in which case you'd want the webpack resolver.

The most help is a PR with a failing test case; otherwise, a repro repo is great.

@nirtamir2
Copy link
Author

nirtamir2 commented Oct 4, 2020

I created this repository to reproduce it.

@ljharb
Copy link
Member

ljharb commented Oct 4, 2020

@nirtamir2 npx eslint . passes, and there's no "lint" or "test" command. How do I reproduce it with that repo?

@nirtamir2
Copy link
Author

I added lint command

@ljharb
Copy link
Member

ljharb commented Oct 5, 2020

@nirtamir2 that fails because it tries to lint files in node_modules; but eslint . does fail now, thanks. I'll take a look.

@ljharb
Copy link
Member

ljharb commented Oct 5, 2020

@nirtamir2 there's no webpack config in that repo. How are svg files importable at all?

@nirtamir2
Copy link
Author

nirtamir2 commented Oct 13, 2020

Sorry for the late response.
It's done by next.js with the babel-plugin-inline-react-svg plugin.
I create this repo just to reproduce it.
Most of the SVG files loading that I use work with svgr, which convert them to react components.
The original issue was in a different react-native project.
I'm not familiar enough with the internals of this plugin, but maybe the problem caused by the glob pattern and better regex matching can help.
It may be because

     pathGroups: [
          {
            pattern: "./assets/*.svg",
            group: "sibling",
            position: "after",
          },
          {
            pattern: "../assets/*.svg",
            group: "sibling",
            position: "after",
          },
        ],

works for svg with the same pattern, but this pattern does not cover all **.svg files

@ljharb
Copy link
Member

ljharb commented Oct 13, 2020

The repro repo would probably have to include next.js then to be helpful :-/

I'm not sure how to proceed here.

@nirtamir2
Copy link
Author

Hi @ljharb
Thank you for the help :}
I reproduced the same issue with bare webpack 5 configuration.
You can see this in the same repository.

@FlorianWendelborn
Copy link

FlorianWendelborn commented Nov 24, 2020

I managed to solve this by using

{
	pattern: '*.scss',
	patternOptions: {
		dot: true,
		nocomment: true,
		matchBase: true,
	},
},

Note: I added nocomment purely because it’s the default for import/order. However, I don’t think it’s actually required.

Also, in theory dot: true should also be optional. The main one is definitely matchBase: true, which instructs minimatch to only attempt to match the filename.

@nirtamir2
Copy link
Author

@FlorianWendelborn Awsome! It works! Thank you so much!
This is my config now for import/order rule:

"import/order": [
      "warn",
      {
        groups: [
          ["builtin", "external"],
          "internal",
          ["parent", "index", "sibling"],
        ],
        pathGroups: [
          {
            pattern: "@hadeshe/**",
            group: "external",
            position: "after",
          },
          {
            pattern: "*.svg",
            patternOptions: {
              dot: true,
              nocomment: true,
              matchBase: true,
            },
            group: "sibling",
            position: "after",
          },
        ],
        pathGroupsExcludedImportTypes: ["builtin"],
        "newlines-between": "always",
        alphabetize: {
          order: "asc",
        },
      },
    ]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants