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

Redo ESLint and TypeScript configuration #30

Merged
merged 2 commits into from
Jun 18, 2024

Conversation

dengr1065
Copy link
Collaborator

Many configuration files in this repository were created a long time ago, then were modified as problems occurred. Now that there is TypeScript support, it makes sense to clean up this mess, at least by making small steps. This configuration is based on strict settings, but most of these are currently disabled - otherwise it would be too hard to work with existing JavaScript code. The downside of this change is pollution of files with warnings and errors, even though they are valid.

  • ESLint/TypeScript upgraded
  • TS configuration is now shared between arbitrary Node scripts, Gulp files and the Electron wrapper
  • A few eslint-disable comments are removed

This was already reviewed by @EmeraldBlock, but expect some breakage (mostly those warnings)

Many configuration files in this repository were created a long time
ago, then were modified as problems occurred. Now that there is
TypeScript support, it makes sense to clean up this mess, at least by
making small steps. This configuration is based on strict settings, but
most of these are currently disabled - otherwise it would be too hard to
work with existing JavaScript code. The downside of this change is
pollution of files with warnings and errors, even though they are valid.

- ESLint/TypeScript upgraded
- TS configuration is now shared between arbitrary Node scripts, Gulp
  files and the Electron wrapper
- A few eslint-disable comments are removed
@dengr1065 dengr1065 merged commit 60b2727 into master Jun 18, 2024
1 check passed
@EmeraldBlock EmeraldBlock self-requested a review June 19, 2024 06:03
Copy link
Collaborator

@EmeraldBlock EmeraldBlock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

retroactively approve a PR challenge

EDIT: nope doesn't work, anyway here's the comments I sent in the Discord for reference, and their resolutions

  • you import path to call a single method on it, yet you import { fileURLToPath } from url !!!
    • habit, due to generic names and possible additional usage in the future
  • https://eslint.org/docs/latest/use/configure/configuration-files#specifying-files-and-ignores
    minimatch supports regex stuff
    or brace expansion
    • this was resolved in the latest commit
  • btw why is tsconfig.json in src now?
  • why isn't the eslint-disable comment at
    // eslint-disable-next-line no-constant-condition
    removed?
    • still relevant and is part of game code
  • for the lint script, for some bizarre reason & acts like a semicolon so eslint . & tsc & tsc -p src suffices
    • turns out this works only on Windows, and the sensible option ; works only on Linux and macOS

eslint.config.js Show resolved Hide resolved
@EmeraldBlock EmeraldBlock deleted the dengr1065/eslint-tsc-revamp branch June 19, 2024 06:11
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

Successfully merging this pull request may close these issues.

2 participants