Conversation
|
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to cd1ebc5 in 8 seconds. Click for details.
- Reviewed
383lines of code in4files - Skipped
1files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_hvwxWPByBYRiudYa
You can customize 
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdated CI matrix and install steps for ESLint versions; bumped ESLint-related dependencies to v10; disabled MDX plugin in ESLint config; and added ESLint-major-version guards and conditional test adjustments in Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/ci.yml (1)
44-46:⚠️ Potential issue | 🔴 CriticalBug: step name references
matrix.nodeinstead ofmatrix.eslint, and the skip condition appears wrong.Two issues:
Step name typo (Line 44):
Install ESLint ${{ matrix.node }}should be${{ matrix.eslint }}.Condition logic (Line 45): The condition
matrix.eslint != 9means this override step runs for both ESLint 8 and ESLint 10. Since the defaulteslintinpackage.jsonis^10.0.0, the override should be skipped for ESLint 10 (not ESLint 9). Running this step for ESLint 10 would needlessly downgrade@graphql-eslint/eslint-pluginto v3,eslint-plugin-svelteto v2,svelteto v3, andvue-eslint-parserto v9 — all of which are only appropriate for the ESLint 8 compatibility path.The condition should likely be
matrix.eslint != 10, and you may also need a separate override set for ESLint 9 vs ESLint 8 if they require different companion package versions.Proposed fix (minimal)
- - name: Install ESLint ${{ matrix.node }} - if: ${{ matrix.eslint != 9 }} - run: pnpm install -D eslint@${{ matrix.eslint }} `@graphql-eslint/eslint-plugin`@3 eslint-plugin-svelte@2 svelte@3 vue-eslint-parser@9 + - name: Install ESLint ${{ matrix.eslint }} + if: ${{ matrix.eslint != 10 }} + run: pnpm install -D eslint@${{ matrix.eslint }} `@graphql-eslint/eslint-plugin`@3 eslint-plugin-svelte@2 svelte@3 vue-eslint-parser@9test/prettier.mjs (1)
530-536:⚠️ Potential issue | 🔴 Critical
eslintPluginMdx.flatis still active in the shared ESLint config despite MDX tests being disabled.Lines 211–293 comment out all MDX test assertions, but the ESLint instance created in
runFixturestill applieseslintPluginMdx.flat(Line 530) and the MDXsettingsblock (Lines 531–536). Ifeslint-plugin-mdxis incompatible with ESLint 10, this will break allrunFixturecalls (HTML, Svelte, Pug, JSON), not just the MDX ones, since they share a single ESLint instance.Consider also commenting out these MDX config entries in the
overrideConfigarray, consistent with the approach ineslint.config.mjs:Proposed fix
- eslintPluginMdx.flat, - { - files: ['*.{md,mdx}'], - settings: { - 'mdx/code-block': true, - }, - }, + // eslintPluginMdx.flat, + // { + // files: ['*.{md,mdx}'], + // settings: { + // 'mdx/code-block': true, + // }, + // },
🤖 Fix all issues with AI agents
In `@package.json`:
- Line 76: Update the dependency entry for the ESLint parser plugin to use the
v9 compatible range by changing the "@eslint/js" dependency version from
"^10.0.1" to "^9.x" (e.g., "@eslint/js": "^9") so it matches ESLint v10's
internal dependency; locate the "@eslint/js" entry in package.json and update
its version string accordingly, then run package manager install to refresh
lockfile.
🧹 Nitpick comments (3)
eslint.config.mjs (1)
5-5: MDX support commented out — consider a TODO or tracking issue.Since the PR title indicates this is exploratory ("testing eslint v10"), leaving commented-out code is fine for now. Consider adding a brief comment explaining why MDX is disabled (e.g., incompatibility with eslint v10) so future readers know the intent and when to re-enable it.
Also applies to: 14-15
test/prettier.mjs (2)
22-22: Unused namespace import if MDX config is also commented out.If the MDX config lines (530–536) are also commented out per the suggestion above, then
eslintPluginMdxon Line 22 becomes unused and should be commented out to matcheslint-mdxon Line 27.Also applies to: 27-27
356-381:invalid-prettierrctests also disabled — is this MDX-related or an ESLint 10 incompatibility?This block doesn't appear directly related to MDX. If it was disabled due to a separate ESLint 10 behavioral change, a brief comment explaining why would help distinguish it from the MDX-related disablements.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 5a04fbd in 8 seconds. Click for details.
- Reviewed
399lines of code in4files - Skipped
1files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_sF2897Pn8XlXRSxC
You can customize
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@test/prettier.mjs`:
- Line 22: The ESLint MDX config is still being applied via eslintPluginMdx.flat
which loads an incompatible MDX plugin for every runFixture; remove or guard the
usage of eslintPluginMdx.flat in the ESLint config block (where the plugin is
merged into the config) so MDX is not applied when running with ESLint v10, and
also comment out or conditionally skip the import "import * as eslintPluginMdx"
if MDX support is disabled; update any references that merge
eslintPluginMdx.flat into the config (the MDX config block used by runFixture)
to be skipped/removed.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed ff8456b in 8 seconds. Click for details.
- Reviewed
481lines of code in4files - Skipped
1files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_soTBWCSCs7L4dMxG
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 54c1bdb in 7 seconds. Click for details.
- Reviewed
485lines of code in4files - Skipped
1files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_uq3EaljtVBoQB9Hc
You can customize
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@test/prettier.mjs`:
- Around line 296-335: The Svelte plugin block must be removed from the active
ESLint configuration because eslint-plugin-svelte v3.14.0 is incompatible with
ESLint v10; locate where the ESLint instance or test harness is configured (the
block referred to as the Svelte config at lines 543–546 in test/prettier.mjs)
and remove any references to the svelte plugin (e.g., removing 'svelte' from
plugins and any 'plugin:svelte/...' entries from extends or parser
configurations) so the plugin is not loaded at test runtime; alternatively swap
those config lines to a conditional that only injects the Svelte config when a
compatible eslint version is detected.
🧹 Nitpick comments (2)
test/prettier.mjs (2)
356-381:invalid-prettierrctests also commented out — what's the reason?This test block doesn't appear to be MDX-related. If it was commented out for a different ESLint v10 incompatibility (e.g., the
nodeTypeor parsing error format changed), a brief comment explaining why would help future maintainers know when to re-enable it.
402-427: TheisESLint9guard may need updating for ESLint v10.
isESLint9is set on line 35 as!eslintUnsupportedApi.FlatRuleTester— this checks whetherFlatRuleTesterwas removed (i.e., merged into mainRuleTester), which happened in ESLint 9. For ESLint 10, this will also betrue, so the name is misleading. Consider renaming to something likeisFlatConfigDefaultorisESLint9Plus.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 1eea598 in 7 seconds. Click for details.
- Reviewed
485lines of code in4files - Skipped
1files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_3YbPrrwRFN5E3QHp
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 6c49f72 in 7 seconds. Click for details.
- Reviewed
451lines of code in4files - Skipped
1files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_1hbOYhNeTd3N4fsk
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 7a9d0bd in 7 seconds. Click for details.
- Reviewed
451lines of code in4files - Skipped
1files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_lJSAN9zYhDpjEFiz
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 4956f34 in 6 seconds. Click for details.
- Reviewed
417lines of code in4files - Skipped
1files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_yxaLBPpeOCoRmo8x
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed e27df88 in 8 seconds. Click for details.
- Reviewed
396lines of code in4files - Skipped
1files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_U8UJ9JGwF3D1y286
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 96d0450 in 7 seconds. Click for details.
- Reviewed
384lines of code in4files - Skipped
1files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_YnmwcA8dVDQTHT4C
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed c3fd96c in 7 seconds. Click for details.
- Reviewed
387lines of code in4files - Skipped
1files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_CPlOtNMDKBHmKJh6
You can customize
|
New version of |
I'm still waiting @wooorm for releasing. :( |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/prettier.mjs (1)
308-345:⚠️ Potential issue | 🟠 MajorSvelte
runFixtureis missing anisESLint10skip guard.The svelte test fixture (lines 309-344) uses
...(isESLint10 ? {} : { nodeType: null })within its test assertions but is not skipped when ESLint v10 is detected. In contrast, the MDX tests are fully wrapped inif (!isESLint10)(line 219) to prevent them from running. The svelte fixture only usessvelteUnsupported(a Node version check) for the skip parameter, which is insufficient.Proposed fix
runFixture( 'eslint-plugin-svelte/*.svelte', [ ... ], - svelteUnsupported, + svelteUnsupported || isESLint10, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/prettier.mjs` around lines 308 - 345, The Svelte test runFixture call should be skipped when ESLint v10 is present; wrap the existing runFixture(...) invocation (the one using svelteUnsupported and the spread ...(isESLint10 ? {} : { nodeType: null })) in the same guard used for MDX tests (if (!isESLint10) { ... }) or alternatively add isESLint10 to the skip condition so the fixture does not run under ESLint 10; update the block containing runFixture, svelteUnsupported, and the ...(isESLint10 ? {} : { nodeType: null }) to ensure the test is not executed when isESLint10 is true.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@test/prettier.mjs`:
- Around line 308-345: The Svelte test runFixture call should be skipped when
ESLint v10 is present; wrap the existing runFixture(...) invocation (the one
using svelteUnsupported and the spread ...(isESLint10 ? {} : { nodeType: null
})) in the same guard used for MDX tests (if (!isESLint10) { ... }) or
alternatively add isESLint10 to the skip condition so the fixture does not run
under ESLint 10; update the block containing runFixture, svelteUnsupported, and
the ...(isESLint10 ? {} : { nodeType: null }) to ensure the test is not executed
when isESLint10 is true.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
.github/workflows/ci.ymleslint.config.mjspackage.jsontest/prettier.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
- eslint.config.mjs
|
eslint-plugin-mdx@3.7.0 has just been released. |
2 participants
Good news! It seems like the existing version of eslint-plugin-prettier works fine with eslint v10.
Our integration tests for
eslint-mdxare failing as that plugin does not yet support eslint v10.Update to test against eslint v10.
@eslint/jsv9 -> v10 and@eslint/jsonv0.14 -> v1.svelte-eslint-parserto v1.5.0 that supports eslint v10eslint-mdxtests in eslint v10, as this plugins for custom file formats does not support eslint 10 yet.