Conversation
rajsite
requested review from
TrevorKarjanis,
atmgrifter00,
jattasNI,
munteannatan and
vivinkrishna-ni
as code owners
|
|
||
| export default metadata; | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars |
There was a problem hiding this comment.
These are kind of annoying to have to suppress. The variable certainly seems to be used on the very next line.
I tried to see if this was a known issue with typescript-eslint but didn't find anything. This was in the same ballpark, but a different case that is marked as fixed. typescript-eslint/typescript-eslint#9679
I'm slightly curious what's going on if you know, but also ok to move along if investigating is expensive.
There was a problem hiding this comment.
Agreed. It is cited in the docs that references only used by types don't count: https://typescript-eslint.io/rules/no-unused-vars/#why-does-this-rule-report-variables-used-only-for-types
And there is long discussions about how they are not convinced about making that configurable: typescript-eslint/typescript-eslint#10266
Alternative 1: We could move them to a different file and export them, then the linter would count them as used. So far we have only done that for shared types so it would be sharing them just for a workaround.
Alternative 2: Could be more clever for this case and derive the type from the enum type PositionState = typeof DropdownPosition[keyof typeof DropdownPosition];. Didn't opt for that because then it doesn't align with the pattern used everywhere else for states. Didn't seem worth diverging.
rajsite
force-pushed
the
eslint-v9-format
branch
from
c932fd5 to
7e79fd8
Compare
# Pull Request ## 🤨 Rationale Adopts eslint v9 🎉 ## 👩💻 Implementation - Switches to ni eslint config packages using flat config - Removes `prettier-eslint` which is in [alpha builds](https://github.com/prettier/prettier-eslint/releases?q=v17&expanded=true) for eslint v9 support, significantly hurt lint performance, and had [major conflicts with eslint](#2718 (comment)). Now that eslint libraries like [stylistic](https://eslint.style/) are dedicated to style rules and eslint supports more formats such as [markdown syntax](https://github.com/eslint/markdown) and [markdown style](https://github.com/ota-meshi/eslint-plugin-markdown-preferences?tab=readme-ov-file) it's possible we don't need to rely on prettier long term. Either way we remove prettier for now to unblock eslint upgrades and can reconsider in the future - Linting performance was significantly improved so simplified the `validate` concurrency scripts to run all lint sequentially in parallel to tests. Now `validate` completes ~100s faster. It's also possible we will get another speed bump enabling [multithreaded eslint](https://eslint.org/blog/2025/08/multithread-linting/) but will save for a follow-up PR. - In the nimble private `eslint-config-nimble` package: - Created a shared `lintNimbleConfig` configuration that is used in each eslint config to make sure the eslint config file itself is linted and to give a spot for very general shared config in follow-up PRs (like markdown or json linting) - Created an angular lint similar to our typescript / component configs as shared config for the angular projects. This resolves issues where extending from a base config in the angular workspace is technically crossing a library boundary and triggered lint errors about reaching across package boundaries. Now we explicitly depend on a shared package. - Angular workspace changes: - The angular workspace root config is now for the config file in root, not the angular projects. - Each angular project uses the shared `angularTypescriptNimbleConfig` and `angularTemplateNimbleConfig` - Root workspace changes: - Added a top-level eslint config file for the config in root (the config file itself and beachball) but could also be used for linting markdown, etc. in the future. - Having a root eslint config also prevents the eslint vscode plugin from getting confused (errors in extension output logs) searching for the eslint config for files in root. ## 🧪 Testing Note, see #2774 for CI results and source changes resulting from this config change. ## ✅ Checklist - [x] I have updated the project documentation to reflect my changes or determined no changes are needed. **A follow-up PR will remove prettier references and docs** --------- Signed-off-by: gokulprasanth-ni <131153319+gokulprasanth-ni@users.noreply.github.com> Co-authored-by: gokulprasanth-ni <131153319+gokulprasanth-ni@users.noreply.github.com> Co-authored-by: rajsite <1588923+rajsite@users.noreply.github.com>
2 participants
Pull Request
🤨 Rationale
This PR is the result of running format and cleaning up the result based on the changes in #2773
Note: This PR is intended to review the source changes that are made and will be merged into the
eslint-v9branch, notmain.👩💻 Implementation
Ran
npm run formatand tweaked the results a but to remove whitespace and address a couple rules that did not have automatic formatters.🧪 Testing
Rely on CI to report no visual diffs and pass.
✅ Checklist