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

[order] How to force React at the top and other issues #1874

Closed
angrybacon opened this issue Aug 8, 2020 · 8 comments
Closed

[order] How to force React at the top and other issues #1874

angrybacon opened this issue Aug 8, 2020 · 8 comments

Comments

@angrybacon
Copy link

I have several issues that I can't fix even with some of the understanding I had from #1665.

The following configuration:

"import/order": ["warn", {
  "alphabetize": {"order": "asc"},
  "groups": [
    ["builtin", "external", "internal"],
    ["parent", "sibling", "index"]
  ],
  "newlines-between": "always",
  "pathGroups": [
    {"group": "builtin", "pattern": "react", "position": "before"},
    {"group": "external", "pattern": "@material-ui/**", "position": "after"},
    {"group": "external", "pattern": "@reach/**", "position": "after"},
    {"group": "external", "pattern": "@saeris/**", "position": "after"}
  ],
  "pathGroupsExcludedImportTypes": [
    "react"
  ]
}],

With the following code:

import React from 'react';
import path from 'path';
import Box from '@material-ui/core/Box';
import CardActionArea from '@material-ui/core/CardActionArea';
import CardContent from '@material-ui/core/CardContent';
import CardMedia from '@material-ui/core/CardMedia';
import Grid from '@material-ui/core/Grid';
import List from '@material-ui/core/List';
import ListItemText from '@material-ui/core/ListItemText';
import Typography from '@material-ui/core/Typography';
import { useTheme } from '@material-ui/core/styles';
import useMediaQuery from '@material-ui/core/useMediaQuery';
import { graphql, useStaticQuery } from 'gatsby';
import AccountEditOutlineIcon from 'mdi-react/AccountEditOutlineIcon';
import CalendarIcon from 'mdi-react/CalendarIcon';
import PropTypes from 'prop-types';

import Card from '../Card';
import Listify from '../Listify';
import Markdown from '../Markdown';
import Prettylink from '../Prettylink';
import useStyles from './styles';

Still gives me the following warnings:

 1:1   warning  There should be at least one empty line between import groups                               import/order
 2:1   warning  There should be at least one empty line between import groups                               import/order
12:1   warning  There should be at least one empty line between import groups                               import/order
13:1   warning  `gatsby` import should occur before import of `path`                                        import/order
14:1   warning  `mdi-react/AccountEditOutlineIcon` import should occur before import of `path`              import/order
15:1   warning  `mdi-react/CalendarIcon` import should occur before import of `path`                        import/order
16:1   warning  `prop-types` import should occur before import of `@material-ui/core/Box`                   import/order

What am I missing here? Also, why is it expecting more newlines since I've set 2 groups?

@ljharb
Copy link
Member

ljharb commented Aug 9, 2020

There needs to be a newline between each group - so, one after react, and one after the node modules (path), and one after the material UI ones. gatsy needs to be sorted with "path", etc.

@angrybacon
Copy link
Author

@ljharb

There needs to be a newline between each group

But if you look at the groups key you'll see that there are only 2 groups. That's how I understood the usage for an array of arrays.

gatsy needs to be sorted with "path", etc.

Why? It's not part of the built-in group is it?

@ljharb
Copy link
Member

ljharb commented Aug 9, 2020

hmm, i see what you mean. if you comment out the pathGroups setting, what happens?

@angrybacon
Copy link
Author

@ljharb

Removing the pathGroups definition, with the same import statements, I get:

1:1  warning  `react` import should occur after import of `prop-types`             import/order
2:1  warning  `path` import should occur after import of `mdi-react/CalendarIcon`  import/order

@kalvis-coinmetro
Copy link

@angrybacon you need

"pathGroupsExcludedImportTypes": ["builtin"],

That works on groups, not regexes. By default it includes also "external", to which react belongs and thus is ignored. This way the react will not be ignored and grouped as you configured - to the top.

@angrybacon
Copy link
Author

angrybacon commented Aug 28, 2020

@kalvisk-blockvis Thanks 👍 that works. Regarding the newlines however, am I wrong to assume that my above setup should require a newline only between the two groups?

{
  "import/order": ["warn", {
    "alphabetize": {"order": "asc"},
    "groups": [
      ["builtin", "external", "internal"],  // <- First group
      ["parent", "sibling", "index"]        // <- Second group
    ],
    "newlines-between": "always",
    "pathGroups": [
      {"group": "builtin", "pattern": "react", "position": "before"},
      {"group": "external", "pattern": "@material-ui/**", "position": "after"},
      {"group": "external", "pattern": "@reach/**", "position": "after"},
      {"group": "external", "pattern": "@saeris/**", "position": "after"}
    ],
    "pathGroupsExcludedImportTypes": ["builtin"]
  }]
}

Currently it treats all custom ordered subgroups as groups requiring a newline between them.

image

Just to be clear, what I wish to accomplish is like in the above screenshot, except with a newline between my useMediaQuery and Card imports.

@kalvis-coinmetro
Copy link

kalvis-coinmetro commented Aug 28, 2020

@angrybacon I also assumed that newlines would be only between groups, not subgroups, but apparently it puts newlines between both groups and subgroups.

I also wish for a nice-to-have feature request that separates this behavior and have two unambiguous config values instead, e.g.

{
  "import/order": ["warn", {
-    "newlines-between": "always",
+    "newlines-between-groups": "always",
+    "newlines-between-subgroups": "never",
  }]
}

@angrybacon
Copy link
Author

@kalvisk-blockvis Oh I see. Thanks for the clarification :-)

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

No branches or pull requests

3 participants