leo-buneev / eslint-plugin-sort-keys-fix

Fork of https://eslint.org/docs/rules/sort-keys that allows automatic fixing
97 stars 23 forks source link

Takes multiple runs to sort #1

Open cjb opened 5 years ago

cjb commented 5 years ago

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.

aarongeorge commented 5 years ago

@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.

namnm commented 4 years ago

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

aress31 commented 4 years ago

Facing the same issue, any update on this?

VictorLeach96 commented 4 years ago

Me too :(

maksimf commented 4 years ago

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

pahan35 commented 4 years ago

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

aarongeorge commented 4 years ago

@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.

pahan35 commented 4 years ago

@aarongeorge feel free to open PR and fix it.

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

pahan35 commented 4 years ago

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.

namnm commented 3 years ago

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

hawtim commented 3 years ago

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

namnm commented 3 years ago

@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

hawtim commented 3 years ago

@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 commented 3 years ago

@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

namnm commented 3 years ago

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

hawtim commented 3 years ago

@namnm thanks a lot. it works.

namnm commented 3 years ago

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