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

Stricter tsconfig and lint for some TypeScript repos #1380

Closed
pixelzoom opened this issue Feb 18, 2023 · 9 comments
Closed

Stricter tsconfig and lint for some TypeScript repos #1380

pixelzoom opened this issue Feb 18, 2023 · 9 comments
Assignees

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Feb 18, 2023

I'm responsible for a growing number of sims and common-code repos that have been converted to TypeScript. Most of them are very clean and exceed the requirements of chipper/eslint/sim_eslintrc.js. and I'd like to keep it that way.

TODO:

(1) Identify what to make stricter, and whether that should be done via tsconfig.json or package.json "eslintConfig".

(2) Identify the correct pattern to use in package.json "eslintConfig". I see this being done in 2 different ways, and I'm not sure if they are correct/equivalent. For example, I added this override to geometric-optics/package.json to prevent // @ts- comments:

"eslintConfig": {
    "extends": "../chipper/eslint/sim_eslintrc.js",
    "overrides": [
      {
        "files": [
          "**/*.ts"
        ],
        "rules": {
          "@typescript-eslint/ban-ts-comment": "error"
        }
      }
    ]
  }

But @zepumph recently added this override to a number of repos to opt-out of "todo-should-have-issue". It's less verbose, but is this also effectively an override? Why is it not in the "eslintConfig.overrides" section?

  "eslintConfig": {
    "extends": "../chipper/eslint/sim_eslintrc.js",
    "rules": {
      "todo-should-have-issue": "off"
    }
  }
@pixelzoom pixelzoom self-assigned this Feb 18, 2023
pixelzoom added a commit to phetsims/models-of-the-hydrogen-atom that referenced this issue Feb 18, 2023
pixelzoom added a commit to phetsims/geometric-optics that referenced this issue Feb 18, 2023
pixelzoom added a commit to phetsims/simula-rasa that referenced this issue Feb 18, 2023
@pixelzoom
Copy link
Contributor Author

... I see this being done in 2 different ways, and I'm not sure if they are correct/equivalent.

I tested and they are equivalent. So in the above commit, I've replaced "eslintConfig.override.rules" with the simpler "eslintConfig.rules" form.

pixelzoom added a commit to phetsims/geometric-optics-basics that referenced this issue Feb 18, 2023
pixelzoom added a commit to phetsims/calculus-grapher that referenced this issue Feb 18, 2023
pixelzoom added a commit to phetsims/geometric-optics that referenced this issue Feb 18, 2023
pixelzoom added a commit to phetsims/simula-rasa that referenced this issue Feb 18, 2023
pixelzoom added a commit to phetsims/models-of-the-hydrogen-atom that referenced this issue Feb 18, 2023
pixelzoom added a commit to phetsims/acid-base-solutions that referenced this issue Feb 18, 2023
pixelzoom added a commit to phetsims/gas-properties that referenced this issue Feb 18, 2023
pixelzoom added a commit to phetsims/beers-law-lab that referenced this issue Feb 18, 2023
pixelzoom added a commit to phetsims/concentration that referenced this issue Feb 18, 2023
pixelzoom added a commit to phetsims/gases-intro that referenced this issue Feb 18, 2023
pixelzoom added a commit to phetsims/equality-explorer-two-variables that referenced this issue Feb 18, 2023
pixelzoom added a commit to phetsims/vector-addition-equations that referenced this issue Feb 18, 2023
pixelzoom added a commit to phetsims/graphing-slope-intercept that referenced this issue Feb 18, 2023
pixelzoom added a commit to phetsims/equality-explorer-basics that referenced this issue Feb 18, 2023
pixelzoom added a commit to phetsims/molecule-polarity that referenced this issue Feb 18, 2023
pixelzoom added a commit to phetsims/equality-explorer that referenced this issue Feb 18, 2023
pixelzoom added a commit to phetsims/natural-selection that referenced this issue Feb 18, 2023
pixelzoom added a commit to phetsims/ph-scale-basics that referenced this issue Feb 18, 2023
pixelzoom added a commit to phetsims/example-sim that referenced this issue Feb 18, 2023
pixelzoom added a commit to phetsims/models-of-the-hydrogen-atom that referenced this issue Feb 21, 2023
pixelzoom added a commit to phetsims/vector-addition-equations that referenced this issue Feb 21, 2023
pixelzoom added a commit to phetsims/equality-explorer-basics that referenced this issue Feb 21, 2023
pixelzoom added a commit to phetsims/graphing-slope-intercept that referenced this issue Feb 21, 2023
pixelzoom added a commit to phetsims/geometric-optics-basics that referenced this issue Feb 21, 2023
pixelzoom added a commit to phetsims/graphing-quadratics that referenced this issue Feb 21, 2023
pixelzoom added a commit to phetsims/acid-base-solutions that referenced this issue Feb 21, 2023
pixelzoom added a commit to phetsims/molecule-polarity that referenced this issue Feb 21, 2023
pixelzoom added a commit to phetsims/equality-explorer that referenced this issue Feb 21, 2023
pixelzoom added a commit to phetsims/natural-selection that referenced this issue Feb 21, 2023
pixelzoom added a commit to phetsims/wave-interference that referenced this issue Feb 21, 2023
pixelzoom added a commit to phetsims/calculus-grapher that referenced this issue Feb 21, 2023
pixelzoom added a commit to phetsims/geometric-optics that referenced this issue Feb 21, 2023
pixelzoom added a commit to phetsims/ph-scale-basics that referenced this issue Feb 21, 2023
pixelzoom added a commit to phetsims/gas-properties that referenced this issue Feb 21, 2023
pixelzoom added a commit to phetsims/concentration that referenced this issue Feb 21, 2023
pixelzoom added a commit to phetsims/beers-law-lab that referenced this issue Feb 21, 2023
pixelzoom added a commit to phetsims/keplers-laws that referenced this issue Feb 21, 2023
pixelzoom added a commit to phetsims/scenery-phet that referenced this issue Feb 21, 2023
@pixelzoom
Copy link
Contributor Author

pixelzoom commented Feb 21, 2023

In the above commits, I moved all TypeScript rules to eslintConfig.overrides.rules in package.json, as recommended by @zepumph. I did this for all repos where it was relevant.

Closing.

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

No branches or pull requests

2 participants