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

Takes multiple runs to sort #1

Open
cjb opened this issue Dec 1, 2018 · 17 comments
Open

Takes multiple runs to sort #1

cjb opened this issue Dec 1, 2018 · 17 comments

Comments

@cjb
Copy link

cjb commented Dec 1, 2018

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
Copy link

@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
Copy link

namnm commented Nov 26, 2019

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

@aress31
Copy link

aress31 commented Dec 15, 2019

Facing the same issue, any update on this?

@VictorLeach96
Copy link

VictorLeach96 commented Dec 15, 2019

Me too :(

@maksimf
Copy link

maksimf commented Mar 22, 2020

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

@pahan35
Copy link
Contributor

pahan35 commented Apr 18, 2020

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
Copy link

aarongeorge commented Apr 21, 2020

@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
Copy link
Contributor

pahan35 commented Apr 21, 2020

@aarongeorge feel free to open PR and fix it.

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

@pahan35
Copy link
Contributor

pahan35 commented Apr 22, 2020

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 referenced this issue in namnm/eslint-plugin-sort-keys Jul 15, 2021
@namnm
Copy link

namnm commented Jul 15, 2021

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
Copy link

hawtim commented Aug 9, 2021

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
Copy link

namnm commented Aug 9, 2021

@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
Copy link

hawtim commented Aug 9, 2021

@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
Copy link

hawtim commented Aug 9, 2021

@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
Copy link

namnm commented Aug 10, 2021

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

namnm referenced this issue in namnm/eslint-plugin-sort-keys Aug 10, 2021
@hawtim
Copy link

hawtim commented Aug 11, 2021

@namnm thanks a lot. it works.

@namnm
Copy link

namnm commented Aug 12, 2021

@hawtim Sorry about that, can we move to namnm#3

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

No branches or pull requests

8 participants