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] Potential regression in sibling and parent behavior #2671

Closed
erunion opened this issue Jan 12, 2023 · 5 comments · Fixed by #2674
Closed

[import/order] Potential regression in sibling and parent behavior #2671

erunion opened this issue Jan 12, 2023 · 5 comments · Fixed by #2674

Comments

@erunion
Copy link

erunion commented Jan 12, 2023

We have a bit of a complex import/order config and are seeing some new weird behavior with ordering on the sibling and parent groups on eslint-plugin-import@2.27.4:

'import/order': [
  'error',
  {
    alphabetize: {
      order: 'asc',
      caseInsensitive: true,
    },
    groups: ['type', 'builtin', 'external', 'internal', 'parent', 'sibling', 'index', 'object'],
    'newlines-between': 'always',
    pathGroups: [
      { pattern: '@Hub', group: 'external', position: 'after' },
      { pattern: '@Hub/**', group: 'external', position: 'after' },
      { pattern: '@Dash/**', group: 'external', position: 'after' },
      { pattern: '@core/**', group: 'external', position: 'after' },
      { pattern: '@routes/**', group: 'external', position: 'after' },
      { pattern: '@ui', group: 'external', position: 'after' },
      { pattern: '@ui/**', group: 'external', position: 'after' },
    ],
    pathGroupsExcludedImportTypes: ['@Hub', '@Dash', '@core', '@routes', '@ui'],
    warnOnUnassignedImports: true,
  },
],

Here's what our code looks like on eslint-plugin-import@2.26.0:

import React from 'react';

import Flex from '@ui/Flex';
import Menu, { MenuItem, MenuDivider } from '@ui/Menu';

import { isPolymorphic } from '../../utils';

import classes from './style.module.scss';
import './style.scss';

And it wants it to be now:

import React from 'react';

import Flex from '@ui/Flex';
import Menu, { MenuItem, MenuDivider } from '@ui/Menu';

import classes from './style.module.scss';
import './style.scss';

import { isPolymorphic } from '../../utils';

However if I swap the the order of parent and sibling in the config to this then the behavior we were seeing on v2.26.0 remains:

groups: ['type', 'builtin', 'external', 'internal', 'sibling', 'parent', 'index', 'object']
@ljharb
Copy link
Member

ljharb commented Jan 12, 2023

I can confirm that this test case passes on v2.26.0 and fails on v2.27.4. It appears to have been broken by #2506. cc @Pearce-Ropion

@Pearce-Ropion
Copy link
Contributor

I can have a look. My assumption is that my "patch" is causing ordering overlaps.

@ljharb
Copy link
Member

ljharb commented Jan 12, 2023

Thanks! That sounds likely.

@cailloumajor
Copy link

cailloumajor commented Jan 13, 2023

I have the same problem: if I update to v2.27.4, I get import/order errors. If I call eslint with --fix, it reorder imports from below:

import { ref, watch } from "vue"

import DashboardMetric from "app/test/cypress/wrappers/DashboardMetricStub.vue"
import LineDashboardWrapper from "app/test/cypress/wrappers/LineDashboardWrapper.vue"
import QPage from "app/test/cypress/wrappers/QPageStub.vue"
import TimelineDisplay from "app/test/cypress/wrappers/TimelineDisplayStub.vue"
import fluxQueryComposable from "composables/flux-query"
import machineDataComposable from "composables/machine-data"
import { makeServer } from "src/dev-api-server"
import { LinkStatus, shiftDurationMillis, staticConfigApi } from "src/global"
import { lineDashboardConfigSchema } from "src/schemas"
import { useCampaignDataStore } from "stores/campaign-data"
import { useCommonLineInterfaceConfigStore } from "stores/common-line-interface-config"
import { useMachineDataLinkStatusStore } from "stores/machine-data-link"

import type { MachineData } from "../LineDashboard.vue"
import type { Server } from "miragejs"
import type { Ref } from "vue"

to below:

import { ref, watch } from "vue"

import DashboardMetric from "app/test/cypress/wrappers/DashboardMetricStub.vue"
import LineDashboardWrapper from "app/test/cypress/wrappers/LineDashboardWrapper.vue"
import QPage from "app/test/cypress/wrappers/QPageStub.vue"
import TimelineDisplay from "app/test/cypress/wrappers/TimelineDisplayStub.vue"
import fluxQueryComposable from "composables/flux-query"
import machineDataComposable from "composables/machine-data"
import { makeServer } from "src/dev-api-server"

import type { MachineData } from "../LineDashboard.vue"

import { LinkStatus, shiftDurationMillis, staticConfigApi } from "src/global"

import type { Server } from "miragejs"

import { lineDashboardConfigSchema } from "src/schemas"

import type { Ref } from "vue"

import { useCampaignDataStore } from "stores/campaign-data"
import { useCommonLineInterfaceConfigStore } from "stores/common-line-interface-config"
import { useMachineDataLinkStatusStore } from "stores/machine-data-link"

I tracked down the commit which introduced this behaviour with git bisect. It pointed me at 7a37f90 (introduced in #2506, as @ljharb stated).

Below is my configuration for import plugin:

const tsConfig = require("./tsconfig.json")
const internalPathPatterns = Object.keys(tsConfig.compilerOptions.paths)
  .filter((pat) => pat.endsWith("/*"))
  .map((pat) => pat.slice(0, -2))
  .join(",")
const internalPathGlob = `{${internalPathPatterns}}/**`

// ...snip

    "import/order": [
      "error",
      {
        "newlines-between": "always",
        alphabetize: { order: "asc" },
        groups: [
          "builtin",
          "external",
          "internal",
          "parent",
          "sibling",
          "index",
          "object",
          "type",
        ],
        pathGroups: [
          {
            pattern: internalPathGlob,
            group: "internal",
          },
        ],
        pathGroupsExcludedImportTypes: ["builtin", "type"],
      },
    ],

// ...snip

@Pearce-Ropion
Copy link
Contributor

I just put up #2674 that should address this

@ljharb ljharb closed this as completed in 0778b03 Jan 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

4 participants