PolicyEngine / policyengine-app

PolicyEngine's free web app for computing the impact of public policy.
GNU Affero General Public License v3.0
41 stars 108 forks source link

Ignore the build directory for `eslint` and restore `eslint` in `npm run fix` #1021

Closed MaxGhenis closed 10 months ago

MaxGhenis commented 10 months ago

Removed in #1013

Per https://github.com/PolicyEngine/policyengine-app/issues/1016#issuecomment-1869646241 we may be missing some fixes without it

The workflow that causes the error is as follows: run make build and then run npm run lint or npm run fix. eslint reports errors inside the build dir, which it should ignore.

abhcs commented 10 months ago

Please dump the contents of .eslintrc.json in your root folder. I have two theories:

  1. Your .eslintrc.json contains more rules, e.g., no-func-assign.
  2. Or, eslint ignores .eslintrc.json for some reason on your system.

In any case, if you work on a fresh clone of the repo with the previous version of npm run fix, i.e., with eslint, there is a strong possibility that you will not see the errors.

MaxGhenis commented 10 months ago

I think I just did npm install eslint and installed the VS Code extension. Here it is:

{
  "env": {
    "browser": true,
    "node": true,
    "es2021": true,
    "jest/globals": true
  },
  "extends": [
    "eslint:recommended",
    "plugin:react/recommended"
    //"plugin:prettier/recommended"
  ],
  "globals": {
    "process": true
  },
  "overrides": [
    {
      "files": ["test/**"],
      "plugins": ["jest"],
      "extends": [
        "eslint:recommended",
        "plugin:jest/recommended"
        //"plugin:prettier/recommended"
      ],
      "rules": { "jest/prefer-expect-assertions": "off" }
    }
  ],
  "parserOptions": {
    "ecmaVersion": "latest",
    "sourceType": "module"
  },
  "plugins": ["react", "react-hooks", "jest", "jsx-a11y"],
  "rules": {
    // suppress errors for missing 'import React' in files
    "react/react-in-jsx-scope": "off",
    "react/prop-types": "off"
  },
  "ignorePatterns": ["**/*.css", "**/*.scss"],
  "settings": {
    "react": {
      "pragma": "React", // Pragma to use, default to "React"
      "fragment": "Fragment", // Fragment to use (may be a property of <pragma>), default to "Fragment"
      "version": "detect" // React version. "detect" automatically picks the version you have installed.
    }
  }
}
abhcs commented 10 months ago

So far we have identical setups. If npm run fix was finding errors in #1016, then npm run lint should also have been finding errors because both use almost the same eslint command. Can you please confirm that npm run lint is also broken for you and post the output if that is the case?

After doing that, please change npm run fix to eslint --debug --ext js,jsx . (locally). Then, run npm run fix 2>&1| tee npm.txt. Then, share npm.txt here or some other way (I am not sure if we can post logs with >10k lines here) npm.txt will contain the debug output, hopefully allowing me to narrow down on the cause of the problem.

MaxGhenis commented 10 months ago

Yes npm run lint throws the same errors as the previous npm run fix:

no-unused-vars
  2:615440  error  Unnecessary escape character: \/                                                                                                                           no-useless-escape
  2:615848  error  Unnecessary escape character: \/                                                                                                                           no-useless-escape
  2:622486  error  Do not access Object.prototype method 'hasOwnProperty' from target object                                                                                  no-prototype-builtins
  2:624333  error  Expected a conditional expression and instead saw an assignment                                                                                            no-cond-assign
  2:647558  error  Do not access Object.prototype method 'hasOwnProperty' from target object                                                                                  no-prototype-builtins

✖ 825 problems (825 errors, 0 warnings)

Here's the npm.txt output: https://gist.github.com/MaxGhenis/ac461af2f2e5a9c1267f9292b9583e0b