feat: configure eslint lines-between-class-members rule repository-wide#1474
Closed
josemontespg wants to merge 19 commits into
Closed
feat: configure eslint lines-between-class-members rule repository-wide#1474josemontespg wants to merge 19 commits into
josemontespg wants to merge 19 commits into
Conversation
- Set up ESLint configs with Google GTS style guide for all TypeScript projects (angular, lit, markdown-it, editor, inspector). - Enabled the lines-between-class-members rule to enforce blank lines between class members across all packages. - Configured quotes rule to allow backtick template literals for any string literals. - Formatted all source files repository-wide. - Added ESLint execution steps to all corresponding CI workflows to enforce linting repository-wide on push/PR. TAG=agy CONV=2a56c296-3b1f-4e6d-9e4a-5b15e8a7504c
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request implements gts (Google TypeScript Style) linting across multiple packages, including angular, lit, markdown-it, editor, and inspector, while adding custom ESLint rule overrides for class member spacing and quote styles. Feedback suggests centralizing these duplicated configurations at the repository root to improve maintainability. Additionally, it is recommended to use TypeScript-aware versions of class member rules to correctly handle method overloads and to ensure custom overrides are placed before the Prettier configuration in the React package to prevent formatting conflicts.
| '@typescript-eslint/no-explicit-any': 'off', | ||
| '@typescript-eslint/no-unsafe-function-type': 'off', | ||
| '@typescript-eslint/no-floating-promises': 'off', | ||
| 'lines-between-class-members': ['error', 'always', {exceptAfterSingleLine: true}], |
Contributor
There was a problem hiding this comment.
For TypeScript projects, the base ESLint rule lines-between-class-members is unaware of TS-specific constructs like method overloads. This can lead to incorrect lint errors when overloads are grouped together without empty lines. It is generally recommended to use a TypeScript-aware version of this rule (such as the one provided by @typescript-eslint/eslint-plugin or @stylistic/eslint-plugin-ts) which correctly handles these cases.
- Replaced the standard lines-between-class-members rule with @stylistic/lines-between-class-members in all 8 configuration files to correctly support TypeScript method overloads. - Installed @stylistic/eslint-plugin in all package devDependencies. - Renamed angular eslint config to eslint.config.mjs to resolve ESM node parsing warnings. - Ran auto-formatting and linter verification across all projects. TAG=agy CONV=2a56c296-3b1f-4e6d-9e4a-5b15e8a7504c
- Placed custom rule overrides before prettierConfig to maintain Prettier compatibility and avoid quote conflicts. - Let Prettier format all ESLint config files. TAG=agy CONV=2a56c296-3b1f-4e6d-9e4a-5b15e8a7504c
…g ts-ignore comments - Added Google LLC Apache 2.0 license headers to all 5 newly created ESLint configurations to satisfy check-license. - Restored // @ts-ignore (along with eslint-disable-next-line @typescript-eslint/ban-ts-comment) in angular v0.8 and v0.9 markdown files to fix Unused @ts-expect-error compiler build errors in CI. TAG=agy CONV=2a56c296-3b1f-4e6d-9e4a-5b15e8a7504c
…ures - Regenerated all 8 package-lock.json files from the public registry to ensure they are 100% in sync with their updated package.json files, resolving the npm ci sync failures in GitHub Actions. TAG=agy CONV=2a56c296-3b1f-4e6d-9e4a-5b15e8a7504c
- Fixed all spacing violations in editor.ts, drawable-canvas.ts, and item-select.ts. - Removed unused import repeat in inspector.ts. - Renamed all remaining newly created eslint configs (lit, markdown-it, editor, inspector) to eslint.config.mjs to resolve ESM Node parsing warnings in CI. TAG=agy CONV=2a56c296-3b1f-4e6d-9e4a-5b15e8a7504c
- Added @angular/common and @angular/platform-browser directly to devDependencies of renderers/angular package.json. - Added --legacy-peer-deps flag to build:renderer npm install script in samples/client/angular/package.json to ensure safe peer resolution in CI environment. - Regenerated corresponding package-lock.json files to ensure they are in sync. TAG=agy CONV=2a56c296-3b1f-4e6d-9e4a-5b15e8a7504c
- Configured the Install top-level deps step in ng_build_and_test.yml to use --legacy-peer-deps, resolving the strict peer dependency version mismatch failures on Angular common/core package resolution in CI. TAG=agy CONV=2a56c296-3b1f-4e6d-9e4a-5b15e8a7504c
…in CI - Pinned all core @angular/* packages in renderers/angular/package.json to the exact same version (21.2.5) to eliminate the ERESOLVE peer dependency mismatches. - Regenerated lockfiles for React, Angular, and Composer to completely align and synchronize all dependencies. - Cleaned up unused imports across the Lit renderer codebase identified by the newly enforced linter. TAG=agy CONV=2a56c296-3b1f-4e6d-9e4a-5b15e8a7504c
…nc lockfiles - Added eslint and typescript-eslint explicitly to renderers/angular, renderers/lit, renderers/markdown, and tools/editor to fix 'Cannot find module '\''eslint'\''' errors during GTS linting in CI. - Updated pnpm-lock.yaml in tools/composer to synchronize dependencies after adding @stylistic/eslint-plugin. - Re-generated and synchronized package-lock.json files across all modified packages. TAG=agy CONV=cce8fafa-0d7c-484f-83ea-e99a1339f6fc
josemontespg
force-pushed
the
eslint-lines-between-class-members
branch
from
98a9b1c to
0332afb
Compare
josemontespg
force-pushed
the
eslint-lines-between-class-members
branch
from
febb309 to
de91811
Compare
Collaborator
Author
|
This approach works, but makes the formatter CI script much slower, because it has to run This is a limitation of eslint. Until we figure out a way to run this in a quick way O(10s), I don't think we should include it in CI |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Configure eslint lines-between-class-members rule repository-wide
This PR configures ESLint with the lines-between-class-members rule (requiring empty lines between class members) repository-wide.
Key Changes:
Fixed #1473