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

not producing same results as prettier-eslint-cli #142

Closed
ci-vamp opened this issue Feb 2, 2023 · 20 comments
Closed

not producing same results as prettier-eslint-cli #142

ci-vamp opened this issue Feb 2, 2023 · 20 comments

Comments

@ci-vamp
Copy link

ci-vamp commented Feb 2, 2023

Describe the bug
i have prettier-eslint vscode set to format on save. it is not producing the same results as the prettier-eslint-cli.

i use the cli to check for formatting differences in CI during a PR.

To Reproduce
Steps to reproduce the behavior:

  1. run prettier-eslint \"**/*.{ts,tsx}\" --write
  2. go into file to see diff
  3. hit save (to trigger format on save)
  4. see a change made

Expected behavior

output of prettier-eslint vscode extension format on save is the same as running the format script:

prettier-eslint \"**/*.{ts,tsx}\" --write

Example Project

happening in a private repo

Screenshots

results from prettier-eslint-cli

prettier-eslint \"**/*.{ts,tsx}\" --write

image

results from prettier-eslint vscode format on save

image

Versions (please complete the following information):

  • VS Code Prettier ESLint extension [e.g. 3.1.0]: v5.0.4
  • Visual Studio Code: [e.g 1.57.1]: 1.74.3 (Universal)
  • Node: [e.g. 12.21.0]: v18.13.0
  • Package Manager [e.g. npm 7.18.1, yarn 1.22.10, pnpm 6.7.6]: yarn 1.22.19
  • prettier: [e.g. 1.9.2]: "prettier": "^2.8.0"
  • eslint: [e.g. 6.8.0]: "eslint": "^8.14.0"

System Specifications (please complete the following information):

  • OS: [e.g. MacOS Big Sur 11.4]: macOS ventura (13.2)
  • Processor: [e.g. Intel i7 6-core 2.6Ghz]: m1 max
  • RAM Size: [e.g. 16 GB]: 64GB

Additional context

`.prettierrc.json

{
  "arrowParens": "always",
  "bracketSpacing": true,
  "jsxBracketSameLine": false,
  "jsxSingleQuote": false,
  "quoteProps": "as-needed",
  "singleQuote": true,
  "semi": true,
  "printWidth": 100,
  "useTabs": false,
  "tabWidth": 2,
  "trailingComma": "es5"
}

.eslintrc.json

{
  "env": {
    "browser": true,
    "es2021": true
  },
  "extends": [
    "plugin:react/recommended",
    "airbnb",
    "airbnb/hooks",
    "plugin:import/errors",
    "plugin:import/warnings",
    "plugin:eslint-comments/recommended",
    "plugin:@typescript-eslint/recommended",
    "plugin:react-hooks/recommended",
    "prettier"
  ],
  "ignorePatterns": [
    "api/*"
  ],
  "parser": "@typescript-eslint/parser",
  "parserOptions": {
    "ecmaFeatures": {
      "jsx": true
    },
    "ecmaVersion": 12,
    "sourceType": "module",
    "project": "./tsconfig.json",
    "tsConfigRootDir": "."
  },
  "plugins": [
    "import",
    "react",
    "@typescript-eslint",
    "react-native",
    "eslint-comments",
    "eslint-plugin-newline-destructuring"
  ],
  "rules": {
    "import/extensions": [
      "error",
      "never"
    ],
    "import/no-unresolved": "error",
    "no-use-before-define": "off",
    "@typescript-eslint/no-use-before-define": "off",
    "react/jsx-filename-extension": [
      2,
      {
        "extensions": [
          ".jsx",
          ".tsx"
        ]
      }
    ],
    "@typescript-eslint/no-unused-vars": ["error", "all"],
    "@typescript-eslint/no-explicit-any": ["error", { "ignoreRestArgs": true }],
    "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
    ],
    "comma-dangle": [
      "error",
      "always-multiline"
    ]
  },
  "settings": {
    "import/parsers": {
      "@typescript-eslint/parser": [
        ".ts",
        ".tsx"
      ]
    },
    "import/resolver": {
      "typescript": {
        "alwaysTryTypes": true, // always try to resolve types under `<root>@types` directory even it doesn't contain any source code, like `@types/unist`
        // Choose from one of the "project" configs below or omit to use <root>/tsconfig.json by default
        // use <root>/path/to/folder/tsconfig.json
        "project": "<root>/tsconfig.json"
      }
    }
  }
}

npx eslint --print-config .eslintrc.json

outputs correctly without error, can add if needed (it is very long output)

@idahogurl
Copy link
Owner

@ci-vamp What does your tsconfig file look like?

@ci-vamp
Copy link
Author

ci-vamp commented Feb 4, 2023

@idahogurl

{
  "compilerOptions": {
    "allowSyntheticDefaultImports": true,
    "jsx": "react-native",
    "lib": ["dom", "esnext"],
    "moduleResolution": "node",
    "allowJs": true,
    "noEmit": true,
    "skipLibCheck": true,
    "resolveJsonModule": true,
    "strict": true,
    "baseUrl": ".",
    "paths": {
      "@features/*": ["features/*"],
      "@utils/*": ["utils/*"],
      "@assets/*": ["assets/*"],
      "@config/*": ["config/*"],
      "@components/*": ["components/*"],
      "@providers/*": ["providers/*"],
      "@hooks/*": ["hooks/*"],
      "@api": ["api/index"],
      "@navigation": ["navigation/index"],
      "@types": ["types/index"],
      "@screens": ["screens/index"]
    }
  },
  "extends": "expo/tsconfig.base"
}

@ci-vamp
Copy link
Author

ci-vamp commented Feb 4, 2023

I also noticed it (the extension) is not respecting these rules about commas and line breaks for assignments (with =). It "corrects" them (after running the CLI) to remove the dangling comma and add the line break back in.

    "operator-linebreak": [
      "error",
      "after",
      {
        "overrides": {
          "=": "none"
        }
      }
    ],
 
    "comma-dangle": [
      "error",
      "always-multiline"
    ]

@ci-vamp
Copy link
Author

ci-vamp commented Feb 5, 2023

@idahogurl i trimmed out everything to a minimal reproducible example repo here.

i cant understand what is causing it. looking at the extension source its a wrapper around prettier-eslint which is the same as prettier-eslint-cli. if theres anything else i can do to help debug please let me know.

thank you for the wonderful extension.

@idahogurl
Copy link
Owner

@ci-vamp I was able to reproduce and fix your issue. The fix is described here:
#27 (comment)

@ci-vamp
Copy link
Author

ci-vamp commented Feb 8, 2023

@idahogurl i tried this and it is still having the same behavior. i have changed it to .eslintrc.js and added the __dirname value.

image

@ci-vamp
Copy link
Author

ci-vamp commented Feb 8, 2023

here is a direct link to the new eslint config file

i am also not getting any errors in the extension output (as suggested by #27)

image

@ci-vamp
Copy link
Author

ci-vamp commented Feb 8, 2023

@idahogurl im sorry it did fix it!

but in case anyone else runs into this i had to:

  1. delete node_modules
  2. close vscode entirely (all windows not just that project)
  3. reinstall node modules
  4. open vscode

brilliant. thank you very much

@ci-vamp ci-vamp closed this as completed Feb 8, 2023
@ci-vamp
Copy link
Author

ci-vamp commented Feb 14, 2023

@idahogurl sorry to bother but it started happening again. i noticed it happening in my main project and went back to the demo repo to see if someone else on my team had modified something.

i did not change any settings since you sorted me out with converting eslint to a js file.

here is a video in action in the demo repo. is there a way to enable debugging/tracing of what the extension is doing? im close to just abandoning the new line destructuring, seems to be the source of all this :[

eslint-demo-720.mov

@ci-vamp ci-vamp reopened this Feb 14, 2023
@idahogurl
Copy link
Owner

idahogurl commented Feb 14, 2023

@ci-vamp Unfortunately there is not a way to turn on debugging. I have thought about that in the past. I'd have to create a setting in the plugin so it could pass it to Prettier ESLint. There is this plugin https://www.npmjs.com/package/eslint-plugin-newline-destructuring. You shouldn't need it though because prettier-eslint is doing it correctly. Strange because my extension is basically a proxy.

You could clone my repository, build the extension in dev mode, and then use the VS Code extension debugger but it's not exactly intuitive. I'll reclone your demo repo, apply the fix and see if I can't reproduce it again.

@ci-vamp
Copy link
Author

ci-vamp commented Feb 14, 2023

@idahogurl ya it's so strange. Especially because it got fixed after your suggestion. But now it randomly came back.

I tried wiping / reinstalling node modules, removing / reinstalling the extension, quitting / refreshing vscode. Nothing worked to sort it out. I’m really perplexed.

That plugin is the one I'm using now. Like you said it works with the CLI but somehow the extension doesn't (despite being a proxy). What a mystery.

If there's anything I can do to help debug please let me know. I’m not familiar with extension dev but know node/js/ts pretty well.

@idahogurl
Copy link
Owner

@ci-vamp Finally figured it out!! Oh it was painful. 😬 The fix is that you had tsconfigRootDir as tsConfigRootDir in your .eslintrc.js

@ci-vamp
Copy link
Author

ci-vamp commented Feb 21, 2023

oh wow what a catch. this did fix nearly all of it. now it is just hung up on import/order rules :[

import-order-720.mov

@idahogurl
Copy link
Owner

@ci-vamp I can't reproduce the issue. It keeps the line for me. Can you add more to the sample repo so I can reproduce it?

@ci-vamp
Copy link
Author

ci-vamp commented Feb 26, 2023

@idahogurl absolutely. i have updated the repo with a sample hooks/ dir (referenced as @hooks via tsconfig alias).

i also included the tsconfig dir change (forgot to commit it last time) and added .vscode/settings.json for consistency.

it is committed in a "broken" state so you can see the effect of saving.

here is a recording of the demo. when i hit save it corrects everything except the import/order rule. i then manually correct the issue and hit save again which causes the extension to revert it back to the failed formatting.

issue-in-demo-720.mov

@ci-vamp
Copy link
Author

ci-vamp commented Feb 26, 2023

also if it helps my partner has introduced a temporary workaround for us using this extension rohit-gohri.format-code-action. they have done a "poor-man's" version of your extension, you can see in the description their motivation was the same as yours.

i am not a fan of it because it has some sort of memory leak / CPU overload where it starts shredding battery life. and, tbh, i have used your extension for a few years now and much prefer to stick with it.

i can confirm that using their extension with prettier-eslint as the base formatter (see our repo settings below) does work as intended. maybe this will help debug. it effectively runs prettier-eslint then eslint fix.

note: they require that editor.formatOnSave be set to false for their extension to work.

{
  "editor.defaultFormatter": "rvest.vs-code-prettier-eslint",
  "editor.formatOnPaste": false, // required
  "editor.formatOnType": false, // required
  "editor.formatOnSave": false, // optional
  "editor.formatOnSaveMode": "file", // required to format on save
  "editor.codeActionsOnSave": ["source.formatDocument", "source.fixAll.eslint"],
  ...
}

@idahogurl
Copy link
Owner

idahogurl commented Feb 26, 2023

@ci-vamp Thank you for your loyalty and patience. You must be a Hufflepuff like me. 😆 I figured out the issue. It's another .eslintrc.js change.
On line 112

        project: '<root>/tsconfig.json',

change to

       project: `${__dirname}/tsconfig.json`,

Also, you don't need "eslint-config-prettier": "^8.5.0", in your package.json or prettier in your .eslintrc.js

extends: [
    'plugin:react/recommended',
    'airbnb',
    'airbnb/hooks',
    'plugin:import/errors',
    'plugin:import/warnings',
    'plugin:eslint-comments/recommended',
    'plugin:@typescript-eslint/recommended',
    'plugin:react-hooks/recommended',
    'prettier',
  ],

The prettier-eslint package runs the your code throughprettier first and then eslint last.

@ci-vamp
Copy link
Author

ci-vamp commented Feb 27, 2023

yer a wezard id @idahogurl

its working wonderfully.

also of note is i had autosave on delay. i know you had that as a suggestion but i think it should be a requirement to use autosave on focus / window change

@ci-vamp ci-vamp closed this as completed Feb 28, 2023
@ci-vamp
Copy link
Author

ci-vamp commented Mar 1, 2023

ok so it randomly started happening again for my partner. only diff we could find was was on vscode 1.75.1

as soon as i updated to 1.75.1 started happening to me. i was about to file a bug report when i realized that even after downgrading back to 1.74.3 it was still happening!

i did a complete wipe of vscode and it didnt fix anything. lost all my settings :[ im dumb

however! i got the fix and it was based on what you had suggested before.

at the very end of the eslintrc.js was this. their own docs suggest // use <root>/path/to/folder/tsconfig.json BUT it has to be the __dirname stlye! (see fixed version below). leaving this here for reference

  settings: {
    'import/parsers': {
      '@typescript-eslint/parser': ['.ts', '.tsx'],
    },
    'import/resolver': {
      typescript: {
        alwaysTryTypes: true, // always try to resolve types under `<root>@types` directory even it doesn't contain any source code, like `@types/unist`
        // Choose from one of the "project" configs below or omit to use <root>/tsconfig.json by default
        // use <root>/path/to/folder/tsconfig.json
        project: `${__dirname}/tsconfig.json`,
      },
    },
  },

@ci-vamp
Copy link
Author

ci-vamp commented Mar 1, 2023

full eslint for reference

and you were right i was able to remove the eslint-config-prettier package and prettier from eslint extensions

module.exports = {
  env: {
    browser: true,
    es2021: true,
  },
  extends: [
    'plugin:react/recommended',
    'airbnb',
    'airbnb/hooks',
    'plugin:import/errors',
    'plugin:import/warnings',
    'plugin:eslint-comments/recommended',
    'plugin:@typescript-eslint/recommended',
    'plugin:react-hooks/recommended',
  ],
  ignorePatterns: ['api/*', '.eslintrc.js', '*.config.ts', '*.config.js'],
  parser: '@typescript-eslint/parser',
  parserOptions: {
    ecmaFeatures: {
      jsx: true,
    },
    ecmaVersion: 12,
    sourceType: 'module',
    project: `${__dirname}/tsconfig.json`,
    tsconfigRootDir: __dirname,
  },
  plugins: [
    'import',
    'react',
    '@typescript-eslint',
    'react-native',
    'eslint-comments',
    'eslint-plugin-newline-destructuring',
  ],
  rules: {
    'max-classes-per-file': ['off'],
    'import/prefer-default-export': ['off'],
    'react/function-component-definition': ['off'],
    'import/extensions': ['error', 'never'],
    'import/no-unresolved': 'error',
    'no-use-before-define': 'off',
    '@typescript-eslint/no-use-before-define': 'off',
    'react/jsx-filename-extension': [
      2,
      {
        extensions: ['.jsx', '.tsx'],
      },
    ],
    '@typescript-eslint/no-unused-vars': ['error', 'all'],
    '@typescript-eslint/no-explicit-any': ['error', { ignoreRestArgs: true }],
    'padding-line-between-statements': [
      'error',
      {
        blankLine: 'always',
        prev: '*',
        next: 'return',
      },
    ],
    '@typescript-eslint/explicit-module-boundary-types': 'off',
    '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,
        maxLength: 100,
        consistent: true,
      },
    ],
    'operator-linebreak': [
      'error',
      'after',
      {
        overrides: {
          '=': 'none',
        },
      },
    ],
    'no-multi-spaces': 'error',
    indent: 'off',
    '@typescript-eslint/indent': ['error', 2],
    'comma-dangle': ['error', 'always-multiline'],
    'consistent-return': 'off',
  },
  settings: {
    'import/parsers': {
      '@typescript-eslint/parser': ['.ts', '.tsx'],
    },
    'import/resolver': {
      typescript: {
        alwaysTryTypes: true, // always try to resolve types under `<root>@types` directory even it doesn't contain any source code, like `@types/unist`
        project: `${__dirname}/tsconfig.json`,
      },
    },
  },
};

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