Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Next gen style: Prettier #214

Merged
merged 11 commits into from
Mar 31, 2025
Merged

Next gen style: Prettier #214

merged 11 commits into from
Mar 31, 2025

Conversation

cwillisf
Copy link
Contributor

Resolves

Proposed Changes

Primary change: implement new style rules for eslint and prettier, similar to those already being used for NGP work

Supporting changes:

  • Upgrade to eslint@^9
  • Use new flat format required by eslint@^9
  • Move flattened legacy configs into legacy/
  • Make a new interface for more flexibly making eslint configurations (see README.md)

Bonus changes:

  • The new configurations can lint JS/TS embedded in MD and HTML files

Reason for Changes

  • Modernize our style rules
  • Reduce maintenance effort by leaning on recommended rule sets more than we did previously

Test Coverage

Tested on the files in this repository. I also created a temporary HTML file to verify that those rules work.

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR modernizes the ESLint and Prettier configurations by upgrading to ESLint v9, switching to the new flat format, refactoring legacy configurations into a dedicated folder, and enhancing the documentation and examples.

  • Upgrade to ESLint v9 and adapt to its new flat configuration format
  • Refactor legacy configuration files into the "legacy/" folder and remove deprecated JS config files
  • Introduce a new interface for generating ESLint and Prettier configurations alongside updated README examples

Reviewed Changes

Copilot reviewed 20 out of 22 changed files in this pull request and generated no comments.

Show a summary per file
File Description
release.config.js Formatting tweaks in the branch configuration
react.js, node.js, es6.js Removal of old configuration files
prettier.config.mjs, lib/prettier.mjs Addition of Prettier configuration with modern settings
lib/**/*.mjs New and refactored ESLint configurations, including legacy support
README.md Updated documentation and usage examples
.github/workflows/*.yml Minor formatting adjustments in workflow files
commitlint.config.mjs Small formatting update
Files not reviewed (2)
  • .prettierignore: Language not supported
  • package.json: Language not supported
Comments suppressed due to low confidence (1)

lib/eslint.mjs:43

  • Avoid shadowing the outer 'globalsIn' parameter by renaming this variable (e.g., to 'globalValue') to improve clarity.
const globalsIn = globals[objOrKey]

Comment on lines -22 to +29
cat <<EOF
Node version: $(node --version)
NPM version: $(npm --version)
github.event.pull_request.head.label: ${{ github.event.pull_request.head.label }}
github.head_ref: ${{ github.head_ref }}
github.ref: ${{ github.ref }}
github.workflow: ${{ github.workflow }}
EOF
cat <<EOF
Node version: $(node --version)
NPM version: $(npm --version)
github.event.pull_request.head.label: ${{ github.event.pull_request.head.label }}
github.head_ref: ${{ github.head_ref }}
github.ref: ${{ github.ref }}
github.workflow: ${{ github.workflow }}
EOF
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Turn on "Hide whitespace" or add "w=1" to the URL to ignore indenting changes :)

Copy link

@georgyangelov georgyangelov left a comment

Choose a reason for hiding this comment

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

Looks great!

Maybe on the NGP side we should eventually update scratch-platform to use this one?

@cwillisf
Copy link
Contributor Author

Maybe on the NGP side we should eventually update scratch-platform to use this one?

I think that would make sense: using this configuration in more places means more opportunities for us to find improvements 😁

The first place I'll be using this is in the "Scratchium" work, and it seems likely that I'll run into issues of some sort there. If so, I hope to find non-disruptive ways to resolve those issues. That's why makePrettierConfig is a function, for example, even though it doesn't (yet?) take any arguments. This potential for an initially bumpy road is the main reason I can think of that you might want to hold off for a minute or two before adopting this.

@cwillisf cwillisf merged commit 3568d96 into master Mar 31, 2025
2 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Mar 31, 2025
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants