feat(echarts): extend peer dependency support for ECharts 6#2580
Conversation
✅ Deploy Preview for ix-storybook canceled.
|
🦋 Changeset detectedLatest commit: 90d0c36 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request upgrades the echarts dependency and peer dependency to ^6.0.0 and updates the test files to use the new version. Feedback on these changes includes requiring a major changeset to document this breaking change, removing commented-out code in basic.html instead of leaving it in the codebase, and pinning the CDN script tags in the test files to a specific version (e.g., 6.0.0) rather than using the major version tag @6 to ensure reproducible test runs.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
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:
📝 WalkthroughWalkthroughThe ChangesECharts v6 dependency upgrade
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
.changeset/afraid-mammals-send.md (1)
1-6: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftAdd migration guidance and GitHub issue reference to the changeset.
This changeset declares a major breaking change but is missing critical information required by the Siemens IX review rules:
Migration guidance: As per path instructions, major changes require migration guidance in the changeset summary. Users need clear instructions that they must upgrade their
echartsdependency to^6.0.0and any breaking API changes they need to handle.GitHub issue reference: The PR description mentions this addresses GitHub Issue
#2561. As per path instructions, include "Fixes#2561" in the changeset summary to close the issue on release.Breaking change explanation: Explain that the
peerDependenciesconstraint now requires echarts v6, which is a breaking change for consumers still on v5.📝 Suggested changeset content
--- '`@siemens/ix-echarts`': major --- -Updated dependencies of `echarts` to support `^6.0.0` +**Breaking Change**: Updated echarts to version 6 + +The `@siemens/ix-echarts` package now requires echarts v6 as a peer dependency. + +**Migration required**: +- Update your echarts dependency to `^6.0.0` in your project +- Review the [echarts v6 migration guide](https://echarts.apache.org/handbook/en/basics/release-note/v6-upgrade-guide/) for any breaking API changes +- Test your charts to ensure compatibility with echarts v6 + +Fixes `#2561`🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.changeset/afraid-mammals-send.md around lines 1 - 6, In the changeset file for `@siemens/ix-echarts` major version, add migration guidance explaining that users must upgrade their echarts dependency to ^6.0.0 and handle any resulting breaking API changes. Include "Fixes `#2561`" in the changeset summary to automatically close the GitHub issue on release. Also add a clear explanation that the peerDependencies constraint now requires echarts v6, which is a breaking change for consumers still using v5.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.changeset/afraid-mammals-send.md:
- Around line 1-6: In the changeset file for `@siemens/ix-echarts` major version,
add migration guidance explaining that users must upgrade their echarts
dependency to ^6.0.0 and handle any resulting breaking API changes. Include
"Fixes `#2561`" in the changeset summary to automatically close the GitHub issue
on release. Also add a clear explanation that the peerDependencies constraint
now requires echarts v6, which is a breaking change for consumers still using
v5.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 882c1742-4182-4340-a363-52c3bade5a67
⛔ Files ignored due to path filters (9)
packages/echarts/tests/echarts.e2e.ts-snapshots/echarts-basic-1-chromium---theme-brand-dark-linux.pngis excluded by!**/*.pngpackages/echarts/tests/echarts.e2e.ts-snapshots/echarts-basic-1-chromium---theme-brand-light-linux.pngis excluded by!**/*.pngpackages/echarts/tests/echarts.e2e.ts-snapshots/echarts-basic-1-chromium---theme-classic-dark-linux.pngis excluded by!**/*.pngpackages/echarts/tests/echarts.e2e.ts-snapshots/echarts-basic-1-chromium---theme-classic-light-linux.pngis excluded by!**/*.pngpackages/echarts/tests/echarts.e2e.ts-snapshots/echarts-more-colors-1-chromium---theme-brand-dark-linux.pngis excluded by!**/*.pngpackages/echarts/tests/echarts.e2e.ts-snapshots/echarts-more-colors-1-chromium---theme-brand-light-linux.pngis excluded by!**/*.pngpackages/echarts/tests/echarts.e2e.ts-snapshots/echarts-more-colors-1-chromium---theme-classic-dark-linux.pngis excluded by!**/*.pngpackages/echarts/tests/echarts.e2e.ts-snapshots/echarts-more-colors-1-chromium---theme-classic-light-linux.pngis excluded by!**/*.pngpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
.changeset/afraid-mammals-send.mdpackages/echarts/package.jsonpackages/echarts/tests/basic.htmlpackages/echarts/tests/more-colors.html
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/echarts/package.json`:
- Line 48: The major-release changeset is missing the required migration
guidance for the new ECharts v6 peer contract. Update the changeset summary
associated with the package.json peer dependency change for echarts to include
concrete upgrade/migration notes, and add “Fixes `#2561`” in the summary if this
release should close that issue automatically.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6a5e4a84-5c6c-453f-9e52-7e18abfa1cd5
📒 Files selected for processing (1)
packages/echarts/package.json
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/echarts/tests/v5/basic.html`:
- Around line 1-13: The basic.html test harness is missing required document
metadata, so update the HTML document used by the v5 ECharts tests to include a
DOCTYPE, a lang attribute on the root element, and a title in the head. Make the
change in the basic.html fixture itself, alongside the existing head markup, and
keep it consistent with the metadata pattern used by more-colors.html so the
screenshot tests driven through echarts.e2e.ts render in standards mode.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1c03f4f0-dd3b-40ab-b0fe-51c90e573f0e
⛔ Files ignored due to path filters (17)
packages/echarts/tests/echarts.e2e.ts-snapshots/echarts-basic-1-chromium---theme-brand-dark-linux.pngis excluded by!**/*.pngpackages/echarts/tests/echarts.e2e.ts-snapshots/echarts-basic-1-chromium---theme-brand-light-linux.pngis excluded by!**/*.pngpackages/echarts/tests/echarts.e2e.ts-snapshots/echarts-basic-1-chromium---theme-classic-dark-linux.pngis excluded by!**/*.pngpackages/echarts/tests/echarts.e2e.ts-snapshots/echarts-basic-1-chromium---theme-classic-light-linux.pngis excluded by!**/*.pngpackages/echarts/tests/echarts.e2e.ts-snapshots/echarts-basic-v5-1-chromium---theme-brand-dark-linux.pngis excluded by!**/*.pngpackages/echarts/tests/echarts.e2e.ts-snapshots/echarts-basic-v5-1-chromium---theme-brand-light-linux.pngis excluded by!**/*.pngpackages/echarts/tests/echarts.e2e.ts-snapshots/echarts-basic-v5-1-chromium---theme-classic-dark-linux.pngis excluded by!**/*.pngpackages/echarts/tests/echarts.e2e.ts-snapshots/echarts-basic-v5-1-chromium---theme-classic-light-linux.pngis excluded by!**/*.pngpackages/echarts/tests/echarts.e2e.ts-snapshots/echarts-more-colors-1-chromium---theme-brand-dark-linux.pngis excluded by!**/*.pngpackages/echarts/tests/echarts.e2e.ts-snapshots/echarts-more-colors-1-chromium---theme-brand-light-linux.pngis excluded by!**/*.pngpackages/echarts/tests/echarts.e2e.ts-snapshots/echarts-more-colors-1-chromium---theme-classic-dark-linux.pngis excluded by!**/*.pngpackages/echarts/tests/echarts.e2e.ts-snapshots/echarts-more-colors-1-chromium---theme-classic-light-linux.pngis excluded by!**/*.pngpackages/echarts/tests/echarts.e2e.ts-snapshots/echarts-more-colors-v5-1-chromium---theme-brand-dark-linux.pngis excluded by!**/*.pngpackages/echarts/tests/echarts.e2e.ts-snapshots/echarts-more-colors-v5-1-chromium---theme-brand-light-linux.pngis excluded by!**/*.pngpackages/echarts/tests/echarts.e2e.ts-snapshots/echarts-more-colors-v5-1-chromium---theme-classic-dark-linux.pngis excluded by!**/*.pngpackages/echarts/tests/echarts.e2e.ts-snapshots/echarts-more-colors-v5-1-chromium---theme-classic-light-linux.pngis excluded by!**/*.pngpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
packages/echarts/tests/basic.htmlpackages/echarts/tests/echarts.e2e.tspackages/echarts/tests/more-colors.htmlpackages/echarts/tests/setup-basic.tspackages/echarts/tests/setup-more-colors.tspackages/echarts/tests/v5/basic.htmlpackages/echarts/tests/v5/more-colors.htmlpackages/echarts/tests/v5/setup-basic.tspackages/echarts/tests/v5/setup-more-colors.ts
varun-srinivasa
changed the title
danielleroux
changed the title
|
4 participants
💡 What is the current behavior?
GitHub Issue Number: #2561
🆕 What is the new behavior?
🏁 Checklist
A pull request can only be merged if all of these conditions are met (where applicable):
pnpm test)pnpm lint)pnpm build, changes pushed)👨💻 Help & support
Summary by CodeRabbit
Release Notes
Dependency Updates
@siemens/ix-echarts, withechartsversion constraints expanded to support 5.x and 6.x.Testing & Compatibility
Demo/Layout Updates