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

unexpected indentation behavior #2

Closed
the-vampiire opened this issue Feb 16, 2022 · 9 comments
Closed

unexpected indentation behavior #2

the-vampiire opened this issue Feb 16, 2022 · 9 comments

Comments

@the-vampiire
Copy link

thank you for writing this plugin. i am not alone in wanting clean destructuring!

i do have a small issue that i cant sort out:

goes from this

const {
  SERVICE_NAME_API_BASE_URL,
  SERVICE_NAME_API_PORT,
  SERVICE_NAME_TOKEN,
} = process.env;

to this ?

const {
SERVICE_NAME_API_BASE_URL,
SERVICE_NAME_API_PORT,
SERVICE_NAME_TOKEN
} =
  process.env;

any idea why it removes the indentation and then puts process.env on a new line?

@urielvan
Copy link
Owner

@the-vampiire Would you please provide a mini showcase to reproduce?

@the-vampiire
Copy link
Author

the-vampiire commented Feb 19, 2022

@urielvan sorry busy week. here is a sample on branch issue-#2-unexpected-indent-behavior

  1. clone
  2. checkout issue-#2-unexpected-indent-behavior
  3. npm run fix:prettier

@urielvan
Copy link
Owner

@the-vampiire I am sorry but it seems everything is working as indented, but the result is not what you want. 😢

according to the doc of prettier-eslint, when you call npm run fix:prettier, prettier is called first, then eslint.

I tried run npx prettier src/**/*.ts --write, which called prettier only, and I got this:

const { SERVICE_NAME_API_BASE_URL, SERVICE_NAME_API_PORT, SERVICE_NAME_TOKEN } =
  process.env;

so prettier prefers to put properties on same line and respect the print width option which is 80 by default.

then I tried run eslint src --ext .ts --fix, and I actually got an error since eslint cannot directly deal with typescript. But if you create a simple .eslintrc.js file it works fine.

module.exports = {
  ignorePatterns: ['*.js'],
  parser: '@typescript-eslint/parser',
  parserOptions: {
    project: 'tsconfig.json',
  },
  plugins: ['newline-destructuring'],
  rules: {
    /**
     * this is required, otherwise indentation will be removed, since this plugin deals with destructuring only
     */
    'indent': ['error', 2],
    'newline-destructuring/newline': 'error',
  },
};

conclusion

to avoid prettier puts process.env on a new line, you can either change the default option or tell prettier (ignore code)[https://prettier.io/docs/en/ignore.html#javascript]. I recommend the latter since it won't affect your other code.

as for indentation, a rule should works.

@the-vampiire
Copy link
Author

how embarassing! im sorry @urielvan when i was copying the files over to the fork i did a cp -r /* and of course that didnt grab the dotfiles lol. pushed up with my prettier/eslint configs in place now

@the-vampiire
Copy link
Author

i have now put in my exact code from my codebase to illustrate the issue. you can see what happens in src/[not]-expected.ts when running npm run fix:prettier

@urielvan
Copy link
Owner

urielvan commented Feb 20, 2022

@the-vampiire eslint-config-prettier turns off both indent and @typescript-eslint/indent. so you have to add the rule manually.

the code in as-expected.ts

const { ALGORAND_ALGOD_API_BASE_URL, ALGORAND_ALGOD_API_PORT, ALGORAND_ALGOD_TOKEN }

the length of this part is 84, which is bigger than the default option of prettier, so it will be separated into multiple lines.

while code in not-expected.ts

const { ALGORAND_KMD_API_BASE_URL, ALGORAND_KMD_API_PORT, ALGORAND_KMD_TOKEN }

its length is 79, so prettier prefers to keep in single line. as I said in previous comment, prettier is called first, so it will move process.env to new line.

when eslint is called, this plugin will only deal with destructuring which means only code between braces, the keyword const and those after equal sign are not included. a simplified explanation is:

// the whole line is a variable declaration
const { prop1, prop2 } = foo.bar;
//      \__________/     \_____/
//           |              |
//     destructuring     member expression

so in your example, process.env will be left intact.

@the-vampiire
Copy link
Author

eslint-config-prettier turns off both indent and @typescript-eslint/indent. so you have to add the rule manually.

i didnt know this. thanks man thats definitely the heart of the issue here

when eslint is called, this plugin will only deal with destructuring which means only code between braces, the keyword const and those after equal sign are not included. a simplified explanation is:

ah this makes a lot of sense. also beautiful ascii example there!

thank you for the info man. you clearly know your linting. would you happen to know what setting i can control to keep the member expression on the same line as the =? in context what i mean is:

instead of

const {
  ...
} =
  process.env;

i get

const {
  ...
} = process.env;

@urielvan
Copy link
Owner

operator-linebreak seems to work.
I tried follow config:

'operator-linebreak': ['error', 'after', { overrides: { '=': 'none' } }]

and you also may need no-multi-spaces

@the-vampiire
Copy link
Author

the-vampiire commented Feb 21, 2022

PERFECT @urielvan! thank you so much man. its wild how such a small formatting can be so frustrating.

added

"operator-linebreak": [
      "error",
      "after",
      { overrides: { "=": "none" } }
    ],
"no-multi-spaces": "error",
"indent": "off",
"@typescript-eslint/indent": ["error", 2]

final .eslintrc.js

module.exports = {
  root: true,
  parser: "@typescript-eslint/parser",
  parserOptions: {
    tsconfigRootDir: __dirname,
    project: "./tsconfig.json",
  },
  env: {
    es6: true,
    jest: true,
  },
  ignorePatterns: ["node_modules", "build", "coverage", ".eslintrc.js", "jest.config.js"],
  plugins: ["import", "eslint-comments", "eslint-plugin-newline-destructuring"],
  extends: [
    "eslint:recommended",
    "plugin:eslint-comments/recommended",
    "plugin:@typescript-eslint/recommended",
    "plugin:import/typescript",
    "prettier",
  ],
  globals: {
    BigInt: true,
    console: true,
    WebAssembly: true,
  },
  rules: {
    "padding-line-between-statements": [
      "error",
      { blankLine: "always", prev: "*", next: "return" },
    ],
    "@typescript-eslint/explicit-module-boundary-types": "off",
    "eslint-comments/disable-enable-pair": [
      "error",
      {
        allowWholeFile: true,
      },
    ],
    "eslint-comments/no-unused-disable": "warn",
    "jest/valid-title": "off",
    "import/order": [
      "error",
      {
        "newlines-between": "always",
        groups: ["builtin", "external", "internal", "parent", "sibling", "index", "object", "type"],
        alphabetize: {
          order: "ignore",
          caseInsensitive: false
        },
      },
    ],
    "newline-destructuring/newline": [
      "error",
      {
        items: 2,
      }
    ],
    "operator-linebreak": [
      "error",
      "after",
      { overrides: { "=": "none" } },
    ],
    "no-multi-spaces": "error",
    "indent": "off",
    "@typescript-eslint/indent": ["error", 2]
  },
};

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

2 participants