Eslint flat config v10 - #18157
Conversation
codyrancher
left a comment
There was a problem hiding this comment.
- We'll want to validate extensions still work (kubewarden can still be loaded and built using a
yarn linkon shell. - I think we'll want Alex to take a look because of extensions
- And either Neil or Phil because of dependabot
| "entities": "4.5.0", | ||
| "eslint": "7.32.0", | ||
| "eslint-config-standard": "16.0.3", | ||
| "eslint": "10.5.0", |
There was a problem hiding this comment.
That is good catch, updated the lock.
| files: ['**/*.{js,ts,vue}'], | ||
| rules: { | ||
| '@typescript-eslint/no-empty-function': 'off', | ||
| // `@typescript-eslint/ban-types` was split in ts-eslint v8; disable successors. |
There was a problem hiding this comment.
It looks like no-wrapper-object-types might be missing from the expanded rule.
https://typescript-eslint.io/rules/ban-types/
Edit: actually it turns out it was up here https://github.com/rancher/dashboard/pull/18157/changes/BASE..eb1779c01d1fca32ddcf667a38438f9f55355710#diff-7c342a32a589a612146568f6a4112d79a7d325c363e835145936a83ca3c76681R126
| 'import/no-named-as-default': 'off', | ||
|
|
||
| // --- ESLint v9 / typescript-eslint v8 upgrade compatibility --- | ||
| // Disabled (config-only, no source changes) to preserve the pre-upgrade green state. |
There was a problem hiding this comment.
I don't understand this comment. Are we saying that this only affected config files? If so, can we just disable rules for those files instead of for the entire project?
There was a problem hiding this comment.
I tried to make the entire changes without touching the code base. That is what means config-only.
There are too many violations on this one. We can handle them on a different PR, remove maybe one by one and start making file changes.
The goal for this PR is update the version of the eslint without having to change the code base.
For example, if I remove
@typescript-eslint/no-require-imports
It will trigger 229 error in 93 files.
So we can do that later on to avoid complexity on this first PR.
| if (!parserServices?.defineTemplateBodyVisitor) { | ||
| return {}; | ||
| } |
There was a problem hiding this comment.
I think this would cause us to fail silently. If it's not available I think we'd just like to throw and then fix it if it's broken.
There was a problem hiding this comment.
Ok! That is a good point, I will remove this.
There was a problem hiding this comment.
It is a little difficult to evaluate equivalency by hand so I used AI to help find the differences.
Added:
Behavior changed:
- object-shorthand
- old:
['warn', 'properties'] - new:
['warn', 'always']
- old:
Were implicit but now explicit:
comma-spacing, array-bracket-spacing, keyword-spacing, object-curly-spacing, rest-spread-spacing, quote-props, spaced-comment, object-property-newline
Replicated:
--ext .js,.ts,.vue->ignores: ['**/*.tsx', '**/*.jsx', '**/*.mjs', '**/*.cjs', '**/.*', '**/.*/**']- reportUnusedDisableDirectives: 'off' -> Had to be added since the default is
warn - eslint:recommended -> js.configs.recommended
- standard + @vue/standard -> neostandard({ ts: true, noStyle: true, noJsx: true })
- plugin:@typescript-eslint/recommended -> tseslint.configs.recommended
- plugin:vue/vue3-recommended -> pluginVue.configs['flat/recommended']
- plugin:cypress/recommended -> pluginCypress.configs.recommended
- plugin:local-rules/all -> { rules: { 'local-rules/v-clean-tooltip': 'error' } }
|
i had a stab of this today, hadn't realised there was a PR up. would like to review as well, at least to work out where i was going wrong |
b4bba5c to
666b496
Compare
rak-phillip
left a comment
There was a problem hiding this comment.
I think this looks great. I only have a single comment with regards to some potential follow-up work that we might want to consider after merging this change.
666b496 to
a8111de
Compare
|
🔄 Auto-Retry Failed Run The Tests workflow run Follow up on the retried run here if needed: https://github.com/rancher/dashboard/actions/runs/30264497383 |
rak-phillip
left a comment
There was a problem hiding this comment.
I think this change looks great. Based on the scope of this change, I suggest getting at least one more approval before merging.
richard-cox
left a comment
There was a problem hiding this comment.
- @aalves08 once a new extension creator package is created can you give linting a quick test?
- i think it would be good for @codyrancher to review again to confirm his comments are good
7472be7
a8111de to
7472be7
Compare
There was a problem hiding this comment.
🔴 Extensions developer env is failing. Repro steps:
- Clone
https://github.com/aalves08/elemental-ui - Checkout
compatibility-tests-version - Do
yarn install - With this PR+branch, go to the
shellfolder and doyarn link - On the folder where you've cloned
https://github.com/aalves08/elemental-ui, doyarn link "@rancher/shell" - Run the dev env with:
API=.... yarn dev
Extension dev env throws multiple errors.
🟢 Elemental extension does build fine (you'll need to run on the current PR the ./scripts/typegen.sh to generate the missing type definitions and on the extension side you do yarn build-pkg elemental
🔴 scaffolding + build (creators package) and yarn dev, using:
Prerequisites
Make sure you're on the PR branch and deps are installed:
cd /Users/aalves/Desktop/PROJECTS/marcelo-dashboard
# Confirm you're on the right branch
git branch --show-current
# Expected: eslint-flat-config-v10
# Install root deps if not already done
yarn install --frozen-lockfile
# Install shell's own deps (needed so the build scripts are available via file: link)
cd shell && yarn install --frozen-lockfile && cd ..Step 1 — Scaffold the extension skeleton
# Create a fresh temp directory
mkdir /tmp/test-ext-build && cd /tmp/test-ext-build
# Run app/init directly — scaffolds the skeleton and copies eslint.config.mjs
node /Users/aalves/Desktop/PROJECTS/marcelo-dashboard/creators/extension/app/init my-ext "0.0.0-local"
cd my-extAt this point you have the skeleton with the new eslint.config.mjs template.
Step 2 — Scaffold the extension package
# Run pkg/init directly — creates pkg/my-ext/ with index.ts, vue.config.js, tsconfig.json
# and adds build-pkg / serve-pkgs / publish-pkgs scripts to the root package.json
node /Users/aalves/Desktop/PROJECTS/marcelo-dashboard/creators/extension/pkg/init my-extStep 3 — Wire @rancher/shell to the local PR branch
The scaffold set @rancher/shell: "^0.0.0-local" which won't resolve from npm.
Replace it with a file: reference pointing to the local checkout:
node -e "
const fs = require('fs');
const pkg = JSON.parse(fs.readFileSync('package.json', 'utf8'));
pkg.dependencies['@rancher/shell'] = 'file:/Users/aalves/Desktop/PROJECTS/marcelo-dashboard/shell';
fs.writeFileSync('package.json', JSON.stringify(pkg, null, 2));
console.log('Done. @rancher/shell now points to local checkout.');
"With Yarn 1.x classic (which the extension scaffold uses via .yarnrc --frozen-lockfile true),
a file: reference installs the package AND hoists all its dependencies into the extension's
own node_modules. This is why neostandard, typescript-eslint, eslint-plugin-vue, etc.
(all in shell/package.json dependencies) will be importable by eslint.config.mjs.
Step 4 — Install dependencies
# --frozen-lockfile=false because we have no lockfile yet for this extension
yarn install --frozen-lockfile=falseAfter this, confirm the ESLint packages are present:
ls node_modules | grep -E "neostandard|typescript-eslint|eslint-plugin-vue|vue-eslint-parser|@eslint"Expected output (all packages should be there):
@eslint
eslint-plugin-vue
neostandard
typescript-eslint
vue-eslint-parser
Step 5 — Lint
./node_modules/.bin/eslint --max-warnings 0 .What to check:
- No
Cannot find module '...'errors — means ESLint package resolution works - No config parse errors — means the flat config syntax is valid
- Zero warnings/errors on the scaffolded source files — means the rules are correct
Step 6 — Build the extension
yarn build-pkg my-extThis runs node_modules/@rancher/shell/scripts/build-pkg.sh my-ext, which:
- Detects that shell is in
node_modules/@rancher/shell/(noshell/folder at root) - Creates a
.shellsymlink insidepkg/my-ext/pointing to the shell location - Runs
vue-cli-service build --target lib pkg/my-ext/index.ts
Expected output:
Building UI Package my-ext
Package name: my-ext-0.1.0
Package version: 0.1.0
Output formats: umd-min
Output directory: /tmp/test-ext-build/my-ext/dist-pkg/my-ext-0.1.0
...
Build complete.
The built artifacts land in dist-pkg/my-ext-0.1.0/.
Troubleshooting
Cannot find module 'neostandard' (or similar) during lint
The ESLint packages did not hoist from shell. Check:
# Is the file: reference resolved?
ls -la node_modules/@rancher/shell
# Is neostandard missing entirely?
ls node_modules | grep neostandardIf missing, try:
yarn add neostandard@0.13.0 typescript-eslint@8.62.0 vue-eslint-parser@10.4.1 \
"@eslint/js@10.0.1" globals@17.7.0 eslint-plugin-vue@10.9.2 \
"eslint-plugin-cypress@4.3.0" eslint-plugin-import@2.32.0 --dev.shell symlink exists but does not point to expected location during build
The build script creates .shell inside pkg/my-ext/ on each run.
If you ran the build before and the symlink is stale, remove it:
rm -f pkg/my-ext/.shell
yarn build-pkg my-extvue-cli-service: command not found or @vue/cli-service not found
@vue/cli-service should hoist from shell's deps. If it doesn't:
ls node_modules/@vue/cli-service
# If missing:
yarn add @vue/cli-service@5.0.8 --devfrozen-lockfile error on install
The .yarnrc in the template sets --frozen-lockfile true by default.
Since there's no lockfile yet, override it:
yarn install --frozen-lockfile=falseCleanup
rm -rf /tmp/test-ext-build7472be7 to
906c7e5
Compare
|
Seems to be ok. For the yarn link + yarn dev But for the rest of the test it works well. The lint as well doesn't work properly on elemental-ui. But that is same for master. I created a way to make sure can support eslint v7 as well. I did some test and seems to be working. |
aalves08
left a comment
There was a problem hiding this comment.
Given the tests I've done using AI (needed a lot of resolving to do because of webpack version mismatch and all) it looks like this is safe to merge on the extensions side. I think it got proven that this should not affect "yarn dev" nor builds.
given that there was already an approval on the PR, I am approving as well.
Massive work @marcelofukumoto . Kudos
|
@aalves08 thank you! I will merge considering previous approval from @richard-cox and @rak-phillip and fixed comments from @codyrancher. Since it is a big change it could affect later on, but I will be fixing as we find them. |
* Update to v9 with deprecated .eslint for minor changes * Update to v9 with updated config * Added eslint-v10 changes * Fixed semver to 7.7.1, the one previously on the yarn.lock * Applied same semver type to lock the expected version * Updated based on review * Compat to es7


IMPORTANT this can only be merged on v2.16.0 Change is too big to merge it now for v2.15.0
This is a big change as well, so we will need more than one review on it
Summary
Fixes #15498
Fixes #16514
Fixes #17965
Migrates ESLint from v7 (legacy
.eslintrc) to v10 flat config, unblocking the eslint-related Dependabot bumps that could not land on the deprecated config format.Occurred changes and/or fixed issues
.eslintrc.js,.eslintrc.default.js,.eslintignore; replaced witheslint.config.mjs(+ sharedeslint.config.base.mjs) for root,pkg/rancher-components, and the extension scaffolding template.typescript-eslint5→8,eslint-plugin-vue9→10,eslint-plugin-jest27→29,eslint-plugin-cypress2→4.standard/@vue/standard(capped at eslint 8) with their flat successorneostandard; re-added theimportrules it drops.lintscripts (dropped removed--extflag) and thejest/no-if→jest/no-conditional-in-testrule rename.Technical notes summary
--ext .js,.ts,.vue+ dotfile ignores via flat-configignores.Areas or cases that should be tested
lint(root) andlint:lib(pkg/rancher-components) pass with--max-warnings 0.yarn create @rancher/extensionthenyarn lintresolves the toolchain transitively from@rancher/shell.eslint --fix-dry-run .produces no changes.Areas which could experience regressions
eslintdirectly with the removed--extflag.@rancher/shellstandalone install/publish —shell/yarn.lockmust be regenerated for the new toolchain (still pinseslint@7.32.0).Screenshot/Video
N/A — tooling-only change, no UI impact.
Checklist
Admin,Standard UserandUser Base