chore: migrate to neostandard #2920#3201
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
JoshMock
left a comment
There was a problem hiding this comment.
Thanks for this contribution, @johns70! I left a few comments and suggestions.
I'll also note that you've dropped the comments disabling no-misused-new, no-extraneous-class and no-unused-vars from many of the code-generated files, but in some of those files the no-unused-vars disable is still present. Because those code files are 100% generated from the Elasticsearch spec, it will be easier to be consistent with what disable rules are kept at the top of each file.
| }) | ||
|
|
||
| export default [ | ||
| // Cargamos la configuración de neostandard |
There was a problem hiding this comment.
Since this repo targets English as its primary language, please ensure any comments are in English as well.
| // Archivos a IGNORAR (Esto limpia la carpeta src/api) | ||
| { | ||
| ignores: [ | ||
| 'src/api/**', |
There was a problem hiding this comment.
While ignoring src/api/ seems helpful, we do want neostandard or eslint to auto-fix simple linting issues when these files are regenerated from the specification; the code generator does its best to output files in a consistent format but some rules around whitespace, line breaks, etc. are fixed by the linter before generated code is committed.
voxpelli
left a comment
There was a problem hiding this comment.
Some comments from my phone on my way to work 🙂
Thanks for the effort!
| { | ||
| plugins: { | ||
| // Esto extrae el plugin de la base para que las reglas de abajo funcionen | ||
| '@typescript-eslint': baseConfig.find(c => c.plugins?.['@typescript-eslint'])?.plugins['@typescript-eslint'] |
There was a problem hiding this comment.
We export the plugins for convenience, use that instead: https://github.com/neostandard/neostandard#usage-of-exported-plugin
|
|
||
| // Archivos a IGNORAR (Esto limpia la carpeta src/api) | ||
| { | ||
| ignores: [ |
There was a problem hiding this comment.
You can specify this as an ignores option to Neostandard: https://github.com/neostandard/neostandard?tab=readme-ov-file#configuration-options
| "@types/split2": "4.2.3", | ||
| "@types/stoppable": "1.1.3", | ||
| "@typescript-eslint/eslint-plugin": "^8.56.1", | ||
| "@typescript-eslint/parser": "^8.56.1", |
There was a problem hiding this comment.
Do not add these directly, you must use the ones included in neostandard due to how ESLint handles deduping in configs
| "license-checker": "25.0.1", | ||
| "minimist": "1.2.8", | ||
| "ms": "2.1.3", | ||
| "neostandard": "^0.12.2", |
There was a problem hiding this comment.
You can upgrade to version 0.13.0 now 🥳
| ts: true | ||
| }) | ||
|
|
||
| export default [ |
There was a problem hiding this comment.
Consider using defineConfig here: neostandard/neostandard#308
| { | ||
| files: ['**/*.ts', '**/*.cts', '**/*.mts', '**/*.tsx'], | ||
| rules: { | ||
| 'import-x/export': 'off' |
| "test:integration": "npm run test:integration-build && env tap run --jobs=1 --reporter=junit --reporter-file=report-junit.xml generated-tests/", | ||
| "lint": "ts-standard src", | ||
| "lint:fix": "ts-standard --fix src", | ||
| "lint": "eslint .", |
There was a problem hiding this comment.
| "lint": "eslint .", | |
| "lint": "eslint", |
This is enough nowadays
|
Hello, I hope you are well, thank you for the corrections and the advice, my apologies for the inconvenience, I will make the change to the code right now |
|
A quick question: since neostandard 0.13 no longer provides the import/export rule, ESLint now throws Definition for rule 'import/export' was not found for the files generated in src/api. Can I remove the /* eslint-disable import/export */ comments from the generated files, or re-add the plugin in the configuration? @voxpelli |
|
@johns70 Remove the disable comments 👍 |
|
I hope you’re doing well. First of all, I want to apologize for how long it has taken me to make progress on this task; I haven’t had much time, and I try to use the minutes I have to move forward with my responsibilities. This task is very important to me. The purpose of this message is to share some difficulties I’ve encountered with the linter regarding the auto-generated code in the lib and esm folders. I’ve noticed that every time I run npm run test or npm run build, the files in lib and esm are regenerated automatically (I assume this is due to rimraf or another build mechanism, especially when running tests). Because of this, when I try to use npx eslint . --fix, many style errors reappear—particularly indentation, semi, camelcase, and no-void. The --fix option cannot preserve changes in files that are regenerated. So far, the only viable solutions I’ve found are: Run npx eslint . --fix within the test script so that the auto-generated code is formatted automatically before running tests. Completely ignore these folders in the ignore, although I’m somewhat hesitant because this could hide unexpected errors or break project consistency. I’m not sure if I’m missing something in the Neostandard documentation that allows the linter to be permissive enough in certain cases with auto-generated code syntax. So far, I haven’t found another solution that is consistent and reliable. My main question: is --fix intended to apply only to src files, or should it also work on generated code? And if it is expected to work on generated files, is there a recommended way to integrate it without having to ignore entire folders or run eslint --fix inside the tests? Thank you for your time and guidance. Best regards, |
This is the correct approach. We only need linting to cover code in |
Head branch was pushed to by a user without write access
|
@johns70 ESLint is failing again, but I expect it will be easy enough for you to clean up. |
|
@JoshMock Hello, how are you, I just corrected the syntax errors, I hope that now you can pass the tests and if there is any mistake or you want something changed, please do not hesitate to tell me and I will do it as soon as I can, we stay in touch |
|
buildkite test this |
|
@JoshMock and @voxpelli Hello guys, I hope you are doing well. I just wanted to tell you thank you very much for your patience and mentorship throughout this whole process. I apologize if my progress was slower than expected 😭, but I really appreciate all your guidance.💚 This task —migrating lde ts-standard to neostandard— was a big challenge for me, and I learned a lot thanks to your feedback and support.🙏 |
|
Absolutely @johns70. Always happy to help coach someone through new technical challenges! Hope to see you come around again with more contributions soon. ❤️ |
|
I will try to improve my skills and thus be able to help more in your projects, thank you very much Ing @JoshMock , see you in another collaboration 😎 |
|
The backport to To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-9.4 9.4
# Navigate to the new working tree
cd .worktrees/backport-9.4
# Create a new branch
git switch --create backport-3201-to-9.4
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 567b9e69e931eada38cf59079c8719245b6cabcc
# Push it to GitHub
git push --set-upstream origin backport-3201-to-9.4
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-9.4Then, create a pull request where the |
|
The backport to To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-9.2 9.2
# Navigate to the new working tree
cd .worktrees/backport-9.2
# Create a new branch
git switch --create backport-3201-to-9.2
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 567b9e69e931eada38cf59079c8719245b6cabcc
# Push it to GitHub
git push --set-upstream origin backport-3201-to-9.2
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-9.2Then, create a pull request where the |
|
The backport to To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-9.3 9.3
# Navigate to the new working tree
cd .worktrees/backport-9.3
# Create a new branch
git switch --create backport-3201-to-9.3
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 567b9e69e931eada38cf59079c8719245b6cabcc
# Push it to GitHub
git push --set-upstream origin backport-3201-to-9.3
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-9.3Then, create a pull request where the |
|
AI Backport Resolver created a backport PR (no conflicts): #3258 |
|
The backport to To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-9.3 9.3
# Navigate to the new working tree
cd .worktrees/backport-9.3
# Create a new branch
git switch --create backport-3201-to-9.3
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 567b9e69e931eada38cf59079c8719245b6cabcc
# Push it to GitHub
git push --set-upstream origin backport-3201-to-9.3
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-9.3Then, create a pull request where the |
|
AI Backport Resolver resolved the conflicts and created a backport PR: #3259 Files with AI-resolved conflicts:
|
|
The backport to To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-9.2 9.2
# Navigate to the new working tree
cd .worktrees/backport-9.2
# Create a new branch
git switch --create backport-3201-to-9.2
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 567b9e69e931eada38cf59079c8719245b6cabcc
# Push it to GitHub
git push --set-upstream origin backport-3201-to-9.2
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-9.2Then, create a pull request where the |
|
AI Backport Resolver resolved the conflicts and created a backport PR: #3260 Files with AI-resolved conflicts:
|
* chore: migrate to neostandard #2920 (#3201) Co-authored-by: Josh Mock <joshua.mock@elastic.co> Co-authored-by: Josh Mock <josh@joshmock.com> (cherry picked from commit 567b9e6) * ci: trigger rebuild --------- Co-authored-by: johns70 <123770161+johns70@users.noreply.github.com> Co-authored-by: Josh Mock <joshua.mock@elastic.co> Co-authored-by: Josh Mock <josh@joshmock.com> Co-authored-by: margaretjgu <margaret.gu@elastic.co>
* chore: migrate to neostandard #2920 (#3201) Co-authored-by: Josh Mock <joshua.mock@elastic.co> Co-authored-by: Josh Mock <josh@joshmock.com> (cherry picked from commit 567b9e6) * ci: trigger rebuild * fix: remove unused loadArrow function from helpers.ts The loadArrow function was introduced by the neostandard migration cherry-pick but is not used on the 9.3 branch, which still uses static imports for apache-arrow. This caused TS6133 compilation errors in CI. Made-with: Cursor --------- Co-authored-by: johns70 <123770161+johns70@users.noreply.github.com> Co-authored-by: Josh Mock <joshua.mock@elastic.co> Co-authored-by: Josh Mock <josh@joshmock.com> Co-authored-by: margaretjgu <margaret.gu@elastic.co>
* chore: migrate to neostandard #2920 (#3201) Co-authored-by: Josh Mock <joshua.mock@elastic.co> Co-authored-by: Josh Mock <josh@joshmock.com> (cherry picked from commit 567b9e6) * ci: trigger rebuild * fix: resolve neostandard migration lint and TS errors - Remove unused loadArrow function from helpers.ts (TS6133) - Remove unused eslint-disable-line comment from client.ts - Add eslint-disable-next-line no-new in test-import.mjs Made-with: Cursor * fix: add missing Bun/Deno runtime detection in meta header The cherry-pick lost the alternative runtime detection lines that append bn= and dn= to the x-elastic-client-meta header, causing the Bun and Deno meta header unit tests to fail. Made-with: Cursor --------- Co-authored-by: johns70 <123770161+johns70@users.noreply.github.com> Co-authored-by: Josh Mock <joshua.mock@elastic.co> Co-authored-by: Josh Mock <josh@joshmock.com> Co-authored-by: margaretjgu <margaret.gu@elastic.co>
@JoshMock and @margaretjgu @voxpelli 🙏
Hello! This is one of the few contributions I have made as a Junior. I did problem #2920 of moving the project from ts-standard to neostandard to support ESLint 9.
I set up the new eslint.config.mjs file and made sure to ignore the src/api/ folder as the guide says. I also fixed some warnings in the tests (like the descriptions of Symbol() and some any types) so that the linter would have 0 errors.
I reviewed everything with npm run test:unit and all 613 tests passed. I hope everything is correct!

Here is a screenshot of the tests passing locally: