eslint-plugin-sort-keys-fix icon indicating copy to clipboard operation
eslint-plugin-sort-keys-fix copied to clipboard

Takes multiple runs to sort

Open cjb opened this issue 6 years ago • 17 comments

Thanks for writing this!

I noticed when running this on our large codebase that it fails to convert larger objects in one invocation. For example, after running eslint --fix on my machine:

const test = () => ({
  z: "z",
  y: "y",
  x: "x",
  w: "w",
  v: "v",
  u: "u",
  t: "t",
  s: "s",
  r: "r",
  q: "q",
  p: "p",
})

is converted to:

const test = () => ({
  q: 'q',
  p: 'p',
  s: 's',
  r: 'r',
  u: 'u',
  t: 't',
  w: 'w',
  v: 'v',
  y: 'y',
  x: 'x',
  z: 'z',
})

.. and then a second invocation performs the remaining swaps to get to the correct result.

cjb avatar Dec 01 '18 06:12 cjb

@leo-buneev any update on this? I'm also noticing this behavior and whilst it's not really a show stopper (I just save multiple times), it would be nice to handle this in one save.

aarongeorge avatar Mar 21 '19 04:03 aarongeorge

Hi guys, any news? I'm getting the same issue and it breaks the CI build.

namnm avatar Nov 26 '19 10:11 namnm

Facing the same issue, any update on this?

aress31 avatar Dec 15 '19 11:12 aress31

Me too :(

VictorLeach96 avatar Dec 15 '19 12:12 VictorLeach96

This plugin is practically useless because of this issue, it's a shame

maksimf avatar Mar 22 '20 14:03 maksimf

Actually this problem is not so critical. It affects you only when you just applied this plugin.

Further, it nearly doesn't affect you.

#2 is way more critical

pahan35 avatar Apr 18 '20 14:04 pahan35

@pahan35 it's not up to you to determine how critical an issue is, nor is it right to try and hi-jack a thread and direct attention away from this issue.

It affects you only when you just applied this plugin

This is not true nor does it make much sense.

This issue affects every person who uses this plugin, as it causes the plugin to not do what is advertised. Yes we can re-run it, but we can no longer be assured that the sorting is actually complete unless we run it over and over again and compare the diffs. At which point, your issue comes into play with the sort order being problematic.

aarongeorge avatar Apr 21 '20 00:04 aarongeorge

@aarongeorge feel free to open PR and fix it.

I've done it for mine in #15 and #17

pahan35 avatar Apr 21 '20 23:04 pahan35

About the problem with multiple runs: it doesn't work from a single run because it runs fix immediately when it faced a problem. eslint does only one pass through the source code. You can't fix all problems in this manner.

To process it correctly you need to save each node on Property event starting from ObjectExpression event and apply fixes only on ObjectExpression:exit or SpreadElement event: sort all of them accordingly to algorithm and run replacement logic.

I can't believe that any of this issue likers, who are really annoyed by this bug, can't write such logic to fix this bug.

pahan35 avatar Apr 22 '20 00:04 pahan35

I fixed this in my fork and also some other critical bugs as well. Look like this repo is not in maintenance anymore, if anyone still interested in this sort keys plugin please try mine: https://github.com/namnm/eslint-plugin-sort-keys

Install: eslint-plugin-sort-keys Rule: sort-keys/sort-keys-fix

namnm avatar Jul 15 '21 10:07 namnm

To anyone still have interesting in this sort, I made a fix in this PR #32 I also published a new package on npm here: https://www.npmjs.com/package/eslint-plugin-sort-keys

hey namnm, i try your new package but still found this problem exist.

this is my eslint config. could you help me?

module.exports = {
  root: true,
  env: {
    node: true,
  },
  parserOptions: {
    parser: 'babel-eslint',
  },
  settings: {
    'import/resolver': 'webpack',
  },
  plugins: ['simple-import-sort', 'sort-keys'],
  extends: [
    'eslint:recommended',
    'plugin:import/errors',
    'plugin:vue/essential',
  ],
  globals: {
    '__BASEURL__': false,
    '__REGION__': false,
    '__RECORD_MOCK__': false,
    'google': false,
    'mapReady': false,
  },
  rules: {
    // override/add rules settings
    'semi': [2, 'always'],
    'comma-dangle': [2, 'always-multiline'],
    'quotes': [2, 'single'],
    'space-before-function-paren': [2, {
      'anonymous': 'never',
      'named': 'never',
      'asyncArrow': 'always',
    }],
    'object-curly-spacing': [2, 'always', { 'arraysInObjects': true, 'objectsInObjects': false }],
    'camelcase': 0,
    'no-console': 0,
    'import/no-unresolved': 1,
    'jsx-quotes': [2, 'prefer-double'],
    // Off the rule sort-imports and import/order to avoid conflicting with simple-import-sort.
    'sort-imports': 0,
    'import/order': 0,
    'simple-import-sort/imports': 2,
    'simple-import-sort/exports': 2,
    'import/first': 2,
    'import/newline-after-import': 2,
    'import/no-duplicates': 2,
  },
  'overrides': [
    {
      'files': ['src/store/index.js', 'src/views/xSchemas.js', 'src/views/xViews.js'],
      'rules': {
        'sort-keys': 'warn',
      },
    },
  ],
};


I try to lint three files which in overrides, it still need to lint multiple times

hawtim avatar Aug 09 '21 13:08 hawtim

@hawtim Hi can you post the file content of:

  1. before lint
  2. after 1st lint
  3. then your expected

So that I can see and debug where the problem is

namnm avatar Aug 09 '21 14:08 namnm

@hawtim Hi can you post the file content of:

  1. before lint
  2. after 1st lint
  3. then your expected

So that I can see and debug where the problem is

@namnm nice to see your reply. my config

module.exports = {
  root: true,
  env: {
    node: true,
  },
  parserOptions: {
    parser: "babel-eslint",
    sourceType: "module",
    ecmaVersion: 2015,
  },
  plugins: [
    "simple-import-sort",
    "sort-keys"
  ],
  extends: ["eslint:recommended", "plugin:import/errors"],
  rules: {
    // override/add rules settings
    semi: [2, "always"],
    "comma-dangle": [2, "always-multiline"],
    quotes: [2, "single"],
    "space-before-function-paren": [
      2,
      {
        anonymous: "never",
        named: "never",
        asyncArrow: "always",
      },
    ],
    "object-curly-spacing": [
      2,
      "always",
      { arraysInObjects: true, objectsInObjects: false },
    ],
    camelcase: 0,
    "no-console": 0,
    "import/no-unresolved": 1,
    "jsx-quotes": [2, "prefer-double"],
    // Off the rule sort-imports and import/order to avoid conflicting with simple-import-sort.
    "sort-imports": 0,
    "import/order": 0,
    "simple-import-sort/imports": 2,
    "simple-import-sort/exports": 2,
    "import/first": 2,
    "import/newline-after-import": 2,
    "import/no-duplicates": 2,
  },
  overrides: [
    {
      files: ["tests/sort-keys/index.js"],
      rules: {
        "sort-keys": "warn",
      },
    },
  ],
};

run with eslint --fix ./tests/sort-keys/index.js, the file content below:

before lint

const alias = 'alias';
const arrow = 'arrow';
const back = 'back';
const boy = 'boy';
const call = 'call';
const cdb = 'cdb';
const dex = 'dex';
const eric = 'eric';
const pickup = 'pickup';
const random = 'random';
const receive = 'receive';
const sss = 'sss';
const station = 'station';
const test = 'test';
const zone = 'zone';
const zss = 'zss';

export default {
  random,
  receive,
  sss,
  station,
  test,
  zone,
  zss,
  alias,
  arrow,
  back,
  boy,
  call,
  cdb,
  dex,
  eric,
  pickup,
};

it does not sort the keys and output this

warning  Expected object keys to be in ascending order. 'alias' should be before 'zss'  sort-keys

is it anything i missed?

hawtim avatar Aug 09 '21 15:08 hawtim

@hawtim Hi can you post the file content of:

  1. before lint
  2. after 1st lint
  3. then your expected

So that I can see and debug where the problem is

I make a demo here: https://github.com/hawtim/eslint-playground

hawtim avatar Aug 09 '21 15:08 hawtim

@hawtim Thanks for the details, please follow these steps:

  • Upgrade eslint-plugin-sort-keys to the latest (which also include the fix for #2 and #22)
  • The rule is not sort-keys, but sort-keys/sort-keys-fix

namnm avatar Aug 10 '21 01:08 namnm

@namnm thanks a lot. it works.

hawtim avatar Aug 11 '21 03:08 hawtim

@hawtim Sorry about that, can we move to https://github.com/namnm/eslint-plugin-sort-keys/issues/3

namnm avatar Aug 12 '21 10:08 namnm