Skip to content

refactor(charts): split into separate entry point#1073

Merged
spike-rabbit merged 1 commit intomainfrom
refactor/charts-spilt-into-multiple-entry-points
Jan 14, 2026
Merged

refactor(charts): split into separate entry point#1073
spike-rabbit merged 1 commit intomainfrom
refactor/charts-spilt-into-multiple-entry-points

Conversation

@akashsonune
Copy link
Member

@akashsonune akashsonune commented Nov 24, 2025

  • Separate entry points for all the chart components
  • Marked SiChartsNgModule as deprecated
  • Examples/docs updated to use separate entry points

Refactor: Charts Split into Multiple Entry Points

This PR restructures the @siemens/charts-ng library from a monolithic package into a modular architecture with separate entry points for each chart component and shared utilities.

Key Changes:

Modular Entry Points:

  • Created 11 independent sub-packages with dedicated ng-package.json configurations:
    • Chart components: cartesian, circle, chart, gauge, progress, progress-bar, sankey, sunburst
    • Shared modules: common, custom-legend, loading-spinner
  • Each sub-package has its own index.ts entry point

Component Architecture:

  • Extracted and refactored SiChartComponentSiChartBaseComponent to a shared common module
  • All chart components now extend SiChartBaseComponent instead of extending each other
  • Chart components use a shared template and styles (si-chart-base.component.*)
  • Each component registers only its required ECharts modules via echarts.use(), enabling per-component code splitting

Centralized Exports:

  • Core types, interfaces, and options consolidated in @siemens/charts-ng/common
  • Main library re-exports all sub-modules via public-api.ts for backward compatibility
  • Public API goldens added for each sub-package documenting their exported surfaces

Updated Imports:

  • Chart components and examples updated to import from specific sub-modules (e.g., @siemens/charts-ng/sankey instead of @siemens/charts-ng)
  • TypeScript path mappings updated to resolve @siemens/charts-ng/* to individual entry points
  • angular.json sourceRoot updated from projects/charts-ng/src to projects/charts-ng
  • Root ng-package.json entry point changed from src/public-api.ts to public-api.ts

Removed Legacy Structure:

  • Old centralized public API files in src/ directory removed
  • Monolithic component index and module aggregation cleaned up

Benefits:

  • Tree-shaking: Consumers can import only needed chart types, reducing bundle size
  • Performance: Each chart type loads only its required ECharts components
  • Maintainability: Clear module boundaries and separation of concerns
  • Flexibility: Independent versioning and updating of chart components

@coderabbitai
Copy link

coderabbitai bot commented Nov 24, 2025

Walkthrough

This PR restructures the charts-ng library's module architecture from a monolithic public API into a modular system with dedicated sub-packages. The base chart component is renamed from SiChartComponent to SiChartBaseComponent and relocated to a common module. All chart-specific components (Cartesian, Circle, Gauge, Progress, ProgressBar, Sankey, Sunburst) are updated to extend the new base component and reference centralized imports from the reorganized module hierarchy. New ng-package.json files are added to each sub-package for independent packaging, and TypeScript configuration paths are updated to support the new import structure. Generated API golden files document the public surfaces of each sub-package, and the main package exports are reorganized to re-export from the modular sub-packages rather than aggregating types directly.

Possibly related PRs

Suggested labels

enhancement, refactoring

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'refactor(charts): split into separate entry point' accurately describes the main change: the PR refactors the charts package by splitting it into multiple entry points, moving components to separate modules (@siemens/charts-ng/*).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor/charts-spilt-into-multiple-entry-points

📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6c9cc88 and 7b4f818.

📒 Files selected for processing (4)
  • projects/charts-ng/cartesian/si-chart-cartesian.component.ts
  • projects/charts-ng/chart/si-chart.component.ts
  • projects/charts-ng/common/echarts.custom.ts
  • projects/charts-ng/progress-bar/si-chart-progress-bar.component.ts
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: akashsonune
Repo: siemens/element PR: 567
File: projects/charts-ng/src/components/si-chart/si-chart.component.ts:604-609
Timestamp: 2025-12-17T08:58:19.300Z
Learning: In projects/charts-ng/src/components/si-chart/si-chart.component.ts, when implementing theme changes with chart.setTheme() that reuses the ECharts instance without disposing it, do not call afterChartInit() again as it will duplicate event listener registrations, leading to memory leaks and multiple event firings.
Learnt from: dr-itz
Repo: siemens/element PR: 1059
File: projects/charts-ng/src/components/si-chart-cartesian/si-chart-cartesian.component.ts:345-352
Timestamp: 2025-11-25T15:07:09.316Z
Learning: In the charts-ng project, the `SubchartGrid` interface includes a `categoryId` property that is part of the public API and is used on the consumer side. This property is not part of the ECharts `GridComponentOption` type, but the cast is safe because ECharts ignores extra properties at runtime.
📚 Learning: 2025-12-17T08:58:19.300Z
Learnt from: akashsonune
Repo: siemens/element PR: 567
File: projects/charts-ng/src/components/si-chart/si-chart.component.ts:604-609
Timestamp: 2025-12-17T08:58:19.300Z
Learning: In projects/charts-ng/src/components/si-chart/si-chart.component.ts, when implementing theme changes with chart.setTheme() that reuses the ECharts instance without disposing it, do not call afterChartInit() again as it will duplicate event listener registrations, leading to memory leaks and multiple event firings.

Applied to files:

  • projects/charts-ng/common/echarts.custom.ts
  • projects/charts-ng/cartesian/si-chart-cartesian.component.ts
  • projects/charts-ng/chart/si-chart.component.ts
  • projects/charts-ng/progress-bar/si-chart-progress-bar.component.ts
📚 Learning: 2025-11-25T15:07:09.316Z
Learnt from: dr-itz
Repo: siemens/element PR: 1059
File: projects/charts-ng/src/components/si-chart-cartesian/si-chart-cartesian.component.ts:345-352
Timestamp: 2025-11-25T15:07:09.316Z
Learning: In the charts-ng project, the `SubchartGrid` interface includes a `categoryId` property that is part of the public API and is used on the consumer side. This property is not part of the ECharts `GridComponentOption` type, but the cast is safe because ECharts ignores extra properties at runtime.

Applied to files:

  • projects/charts-ng/common/echarts.custom.ts
  • projects/charts-ng/cartesian/si-chart-cartesian.component.ts
  • projects/charts-ng/chart/si-chart.component.ts
  • projects/charts-ng/progress-bar/si-chart-progress-bar.component.ts
📚 Learning: 2025-12-17T04:34:55.597Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1173
File: src/app/examples/si-charts/interactive/interactive.ts:5-5
Timestamp: 2025-12-17T04:34:55.597Z
Learning: Guideline: In Angular projects using OnPush change detection (zoneless mode), you only need to call ChangeDetectorRef.markForCheck() for asynchronous operations (e.g., setTimeout, setInterval, Promises, Observables) where a change is not automatically detected. Event handlers triggered by Angular template bindings (click, input, etc.) will trigger change detection automatically and do not require explicit markForCheck() calls. Apply this broadly to TS files within Angular components/services that use OnPush; use markForCheck() selectively for async work where change detection wouldn’t run otherwise.

Applied to files:

  • projects/charts-ng/common/echarts.custom.ts
  • projects/charts-ng/cartesian/si-chart-cartesian.component.ts
  • projects/charts-ng/chart/si-chart.component.ts
  • projects/charts-ng/progress-bar/si-chart-progress-bar.component.ts
📚 Learning: 2025-12-04T11:54:31.132Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1133
File: projects/element-ng/tour/si-tour.service.spec.ts:21-26
Timestamp: 2025-12-04T11:54:31.132Z
Learning: In the siemens/element repository, all components are standalone by default and do not require the explicit `standalone: true` flag. Components should be added to the `imports` array in TestBed configuration, not the `declarations` array.

Applied to files:

  • projects/charts-ng/chart/si-chart.component.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (6)
projects/charts-ng/common/echarts.custom.ts (1)

1-19: Modular echarts configuration approach is sound.

Centralizing the common echarts registration (renderers and universal components like Title/Tooltip) in a shared module is a clean approach. This allows each chart component to register only its specific chart types and features via echarts.use(), enabling effective tree-shaking.

projects/charts-ng/chart/si-chart.component.ts (1)

5-63: Clean wrapper component with comprehensive chart support.

The refactored SiChartComponent correctly:

  • Imports from the new modular package structure (@siemens/charts-ng/common, @siemens/charts-ng/custom-legend, etc.)
  • Registers the full set of chart types and components via echarts.use() for general-purpose usage
  • Extends SiChartBaseComponent with proper template/style references

This aligns well with the PR's goal of enabling tree-shaking while preserving full functionality for consumers who need all chart types.

projects/charts-ng/progress-bar/si-chart-progress-bar.component.ts (2)

6-21: Excellent minimal ECharts registration for tree-shaking.

This component demonstrates the ideal pattern from the refactor:

  • Imports only the necessary types (BarSeriesOption, ChartXAxis, ChartYAxis) from the common module
  • Registers only BarChart, GridComponent, and LegacyGridContainLabel via echarts.use()

This targeted registration ensures consumers using only the progress-bar chart won't bundle unnecessary chart types like Pie, Gauge, or Sankey.


23-29: Base component pattern correctly applied.

The component properly extends SiChartBaseComponent and references the shared template/styles from the common module. The selector remains unchanged (si-chart-progress-bar), preserving API compatibility.

projects/charts-ng/cartesian/si-chart-cartesian.component.ts (2)

6-61: Well-scoped ECharts registration for cartesian charts.

The component correctly:

  • Imports from modular @siemens/charts-ng/* packages
  • Registers only cartesian-relevant chart types (Bar, Line, Scatter, Candlestick, Heatmap) — excluding non-cartesian types like Pie, Gauge, Sankey, and Sunburst
  • Includes LegacyGridContainLabel which is specifically needed for grid-based charts (as noted in past reviews, this belongs here rather than in the common module)

This addresses the past review's suggestion to move grid-specific features to cartesian components.


63-69: Base component inheritance correctly established.

The component properly extends SiChartBaseComponent, implements OnChanges, and uses the shared template/styles. The selector remains si-chart-cartesian, preserving backward compatibility.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@akashsonune akashsonune changed the title refactor(charts/custom-legend): split into separate entry point refactor(charts): split into separate entry point Nov 24, 2025
@akashsonune akashsonune force-pushed the refactor/charts-spilt-into-multiple-entry-points branch 2 times, most recently from a70c6af to 77dbc39 Compare November 24, 2025 16:47
@github-actions
Copy link

github-actions bot commented Nov 24, 2025

@akashsonune akashsonune force-pushed the refactor/charts-spilt-into-multiple-entry-points branch 9 times, most recently from 7cdd965 to 702b3df Compare November 25, 2025 16:42
@akashsonune
Copy link
Member Author

@dr-itz can you provide some feedback. not everything is moved yet. its just the si-chart, common stuff, sankey and sunburst. For si-chart,I need to find a way to import all the chart types from echarts

@akashsonune akashsonune force-pushed the refactor/charts-spilt-into-multiple-entry-points branch 8 times, most recently from 49f8ca2 to 34885bd Compare December 2, 2025 17:55
@akashsonune akashsonune requested a review from dr-itz December 2, 2025 18:09
@akashsonune
Copy link
Member Author

@dr-itz can you take a look

@akashsonune akashsonune force-pushed the refactor/charts-spilt-into-multiple-entry-points branch 2 times, most recently from 90d4626 to f1cd782 Compare December 4, 2025 10:31
@dr-itz
Copy link
Contributor

dr-itz commented Dec 8, 2025

I like it a lot that only the required echarts parts are loaded for each component. That could significantly reduce loading time and memory consumption in actually a lot of cases 💯

@akashsonune akashsonune force-pushed the refactor/charts-spilt-into-multiple-entry-points branch from d1df6c7 to 4c073f1 Compare December 9, 2025 09:56
@akashsonune akashsonune force-pushed the refactor/charts-spilt-into-multiple-entry-points branch from c6c913d to 6c9cc88 Compare December 22, 2025 11:26
@akashsonune
Copy link
Member Author

@dr-itz @spike-rabbit may you take a look again

@dr-itz
Copy link
Contributor

dr-itz commented Jan 5, 2026

apart from the conflicts, LGTM 👍

@akashsonune akashsonune requested a review from a team as a code owner January 13, 2026 09:30
@akashsonune akashsonune force-pushed the refactor/charts-spilt-into-multiple-entry-points branch 3 times, most recently from 45421e2 to 0f38196 Compare January 13, 2026 10:27
@akashsonune akashsonune added this to the 49.0.0 milestone Jan 13, 2026
@akashsonune akashsonune added the feature Marks feature requests and feature implementations label Jan 13, 2026
@akashsonune
Copy link
Member Author

@spike-rabbit can you check and merge

@akashsonune akashsonune force-pushed the refactor/charts-spilt-into-multiple-entry-points branch 3 times, most recently from 2b29e7b to faa707d Compare January 13, 2026 13:05
@akashsonune
Copy link
Member Author

I guess we need to wait for #1304 for the playwright to pass

@akashsonune akashsonune force-pushed the refactor/charts-spilt-into-multiple-entry-points branch 2 times, most recently from 53d9987 to e1c6e66 Compare January 13, 2026 15:19
@akashsonune
Copy link
Member Author

@spike-rabbit @dr-itz Can you check and merge. Since this is a big PR, shall we keep the commits as is, or do you want me to squash them?

@spike-rabbit
Copy link
Member

@akashsonune please squash, then I will merge

@akashsonune akashsonune force-pushed the refactor/charts-spilt-into-multiple-entry-points branch from fd5faa9 to 7eda2a8 Compare January 14, 2026 09:40
@akashsonune
Copy link
Member Author

@akashsonune please squash, then I will merge

Done, please check

Copy link
Member

@spike-rabbit spike-rabbit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

DEPRECATED: `SiChartsNgModule` is deprecated, import individual components instead. Starting with v49, separate entry points are available for each component, allowing applications to import components from specific entry points, which helps reduce the application bundle size.
@spike-rabbit spike-rabbit force-pushed the refactor/charts-spilt-into-multiple-entry-points branch from 7eda2a8 to 50dce3e Compare January 14, 2026 10:05
@spike-rabbit spike-rabbit enabled auto-merge (rebase) January 14, 2026 10:05
@github-actions
Copy link

Code Coverage

@spike-rabbit spike-rabbit merged commit 06ca45e into main Jan 14, 2026
10 checks passed
@spike-rabbit spike-rabbit deleted the refactor/charts-spilt-into-multiple-entry-points branch January 14, 2026 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Marks feature requests and feature implementations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants