Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

object-literal-sort-keys is nearly (but not quite) alphabetic #1924

Closed
bencoveney opened this issue Dec 23, 2016 · 2 comments · Fixed by #2592
Closed

object-literal-sort-keys is nearly (but not quite) alphabetic #1924

bencoveney opened this issue Dec 23, 2016 · 2 comments · Fixed by #2592

Comments

@bencoveney
Copy link
Contributor

bencoveney commented Dec 23, 2016

Bug (Suggestion?) Report

  • TSLint version: 4.2.0
  • TypeScript version: 2.0.7
  • Running TSLint via: CLI

TypeScript code being linted

const expected = {
  "Chiefs": "#3996D9",
  "CLG": "#008FEA",
  "CLG Red": "#DC0031",
  "Cloud9": "#1D9DD8",
  "compLexity": "#870000",
  "Conquest": "#D08685",
  "CPH Wolves": "#FCBA2F",
  "Crowns": "#E3DB9A",
  "CSGL": "#DB8328",
  "CyberZen": "#BC3728",
}

with tslint.json configuration:

{
  "extends": "tslint:recommended",
  "rules": {
    "no-console": false
  }
}

Actual behavior

The following ordering is asserted by the rule:

const actual = {
  "CLG": "#008FEA",
  "CLG Red": "#DC0031",
  "CPH Wolves": "#FCBA2F",
  "CSGL": "#DB8328",
  "Chiefs": "#3996D9",
  "Cloud9": "#1D9DD8",
  "Conquest": "#D08685",
  "Crowns": "#E3DB9A",
  "CyberZen": "#BC3728",
  "compLexity": "#870000",
}

Expected behavior

For context I have an object literal that I am using as a data store where the keys are CSGO team names. The code above is a sample but the real object is about 100 entries. Storing data in literals like this is not a great idea admittedly but the implication is that the keys can contain pretty much any character (for example capital letters) anywhere in the string.

The actual code sample is the order specified by TSLint. It looks like the rule compares strings by just doing > and JavaScript sorts capitals first IIRC which is what is giving the "almost" alphabetical result.

Another simpler example would be:

// What the rule wants
const actualAgain = {
  array: [],
  objList: [{}, {}],
  object: {},
}

// Alphabetical
const expectedAgain = {
  array: [],
  object: {},
  objList: [{}, {}],
}

The result is kind of logical but unexpected. If I were going through the code above and sorting my literals I would do it alphabetically and the fact that the rule told me otherwise would be confusing, it almost reduces you to trial and error unless you understand the comparison logic being used.

So my question is

  • Should it ideally be sorting alphabetically? (I am thinking .toUpperCase() on both before comparison)
  • If not by default should it be an option?

Also thinking now about the iffy sample I've given, would it be good to subject object literal keys to the same kind of checks performed on identifiers? That is probably a different issue though.

@nchen63
Copy link
Contributor

nchen63 commented Dec 23, 2016

Yeah, I don't think it should consider case when sorting

@adidahiya
Copy link
Contributor

Should be pretty easy to change the behavior to be case-insensitive. Change this line to:

const key = keyNode.text.toLowerCase();

... but this would unfortunately be a breaking change. Best thing would be to add this as a "ignore-case" rule option and change the default in v5.0

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

Successfully merging a pull request may close this issue.

3 participants