Charts: Reorganise shared, hooks and utils #44971

adamwoodnz · 2025-08-27T04:46:42Z

Fixes CHARTS-86: Charts: restructure shared/utils/hooks directories

Proposed changes:

No functional changes, only moves:

Moves everything out of components/shared which has become a bit of a dumping ground. All files and folders moved to either components, hooks or utils. Components by definition are shared/reusable, so they have all been moved into a flat structure.
Implements modern React conventions by moving hooks from utils to separate files prefixed with use-
Moves any tests to a nested test dir
Adds index files to hooks and utils for better devex, and updates imports
Renames use-chart-legend-data to use-chart-legend-items for accuracy
Renames things with global-chart- to global-charts- for consistency

Other information:

Have you written new tests for your changes, if applicable?
Have you checked the E2E test CI results, and verified that your changes do not break them?
Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

Go to http://localhost:50240/?path=/docs/js-packages-charts and ensure the storybook runs
Smoke test the storybook for runtime errors due to misconfigured imports
Browse the source in your IDE or GitHub, and observe the flat file structure. Look for inconsistencies like tests instead of test, hooks not following naming conventions, etc.

github-actions · 2025-08-27T04:47:26Z

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

To test on WoA, go to the Plugins menu on a WoA dev site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin (Jetpack), and enable the charts-86-charts-restructure-sharedutilshooks-directories branch.
To test on Simple, run the following command on your sandbox:

bin/jetpack-downloader test jetpack charts-86-charts-restructure-sharedutilshooks-directories

Interested in more tips and information?

In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
Read more about our development workflow here: PCYsg-eg0-p2
Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

github-actions · 2025-08-27T04:47:41Z

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

✅ Include a description of your PR changes.
✅ Add a "[Status]" label (In Progress, Needs Review, ...).
✅ Add a "[Type]" label (Bug, Enhancement, Janitorial, Task).
✅ Add testing instructions.
✅ Specify whether this PR includes any changes to data or privacy.
✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

Follow this PR Review Process:

Ensure all required checks appearing at the bottom of this PR are passing.
Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
You can use GitHub's Reviewers functionality to request a review.
When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

If you have questions about anything, reach out in #jetpack-developers for guidance!

jp-launch-control · 2025-08-27T04:59:49Z

Code Coverage Summary

Coverage changed in 1 file.

File	Coverage	Δ%	Δ Uncovered
projects/js-packages/charts/src/providers/chart-context/global-charts-provider.tsx	32/32 (100.00%)	0.00%	0 💚

43 files are newly checked for coverage. Only the first 5 are listed here.

File	Coverage
projects/js-packages/charts/src/utils/format-metric-value.ts	8/13 (61.54%) 💚
projects/js-packages/charts/src/components/line-chart/private/line-chart-annotation-label-popover.tsx	16/24 (66.67%) 💚
projects/js-packages/charts/src/components/private/chart-composition/chart-html.tsx	2/3 (66.67%) 💚
projects/js-packages/charts/src/components/private/chart-composition/chart-svg.tsx	2/3 (66.67%) 💚
projects/js-packages/charts/src/components/legend/hooks/use-chart-legend-items.ts	23/29 (79.31%) 💚

Full summary · PHP report · JS report

adamwoodnz · 2025-08-27T05:31:32Z

projects/js-packages/charts/src/components/shared/stories/legend-stories.tsx

- * @param options.customArgTypes - Override default legend positioning argTypes if needed
- * @return Object containing story meta and story objects for legend positioning
- */
-export function createLegendStories< T extends Record< string, unknown > >(


This isn't used

adamwoodnz · 2025-08-27T05:32:25Z

projects/js-packages/charts/src/utils/get-longest-tick-width.ts

@@ -23,10 +23,3 @@ export const getLongestTickWidth = < T extends AnyD3Scale >(

 	return getStringWidth( longestTick, labelStyle );
 };
-
-export const isSafari = () => {


Moved to it's own file

adamwoodnz · 2025-08-29T04:52:59Z

projects/js-packages/charts/src/index.ts

-// Utilities
-export { mergeThemes } from './utils/merge-themes';
-
-// Hooks
-export { useDeepMemo, useGlobalChartTheme, useChartMouseHandler, useXYChartTheme } from './hooks';
-
-// LeaderboardChart utilities
-export { formatMetricValue } from './components/leaderboard-chart';
-
 // Types
 export type * from './types';
 export type * from './visx/types';

 export type { LineStyles, GridStyles, EventHandlerParams } from '@visx/xychart';
-
-export type { RenderLineStartGlyphProps } from './components/line-chart/line-chart';
-export type { LegendProps, BaseLegendProps, ChartLegendOptions } from './components/legend';
-export type { ConversionFunnelChartProps, FunnelStep } from './components/conversion-funnel-chart';


All exported from their respective component indexes

adamwoodnz · 2025-08-29T05:13:37Z

Todo:

Try installing build in woo-analytics and calypso to check imports
Move remaining line-chart types to the index
Document build warnings for discussion

adamwoodnz · 2025-09-01T00:42:53Z

projects/js-packages/charts/src/hooks/index.ts

When running a build Rollup warns us about using these indexes:

(!) Export "useChartRegistration" of module "src/providers/chart-context/hooks/use-chart-registration.ts" was reexported through module "src/providers/chart-context/index.ts" while both modules are dependencies of each other and will end up in different chunks by current Rollup settings. This scenario is not well supported at the moment as it will produce a circular dependency between chunks and will likely lead to broken execution order.
Either change the import in "src/components/pie-chart/pie-chart.tsx" to point directly to the exporting module or do not use "output.preserveModules" to ensure these modules end up in the same chunk.
src/components/pie-chart/pie-chart.tsx

It sees them as circular dependencies, however Claude tells me this is a false positive:

Analysis
Based on my examination of the code, this is indeed a false positive warning. Here's why:

Dependency Flow Analysis
pie-chart.tsx imports useChartRegistration from src/providers/chart-context/index.ts
index.ts re-exports useChartRegistration from ./hooks/use-chart-registration
use-chart-registration.ts imports useGlobalChartsContext from ./use-global-charts-context
use-global-charts-context.ts imports GlobalChartsContext from ../global-charts-provider

Key Points:

No actual circular dependency exists - There's a clear one-way dependency flow:
pie-chart.tsx → chart-context/index.ts → use-chart-registration.ts → use-global-charts-context.ts → global-charts-provider.tsx

No imports back to index.ts - None of the files in the chart-context directory import from the index.ts file, which would be required for a true circular dependency.

Rollup's preserveModules configuration - The warning is triggered by Rollup's preserveModules: true setting in your config (line 38 in rollup.config.mjs). This setting creates separate chunks for each module, and Rollup gets confused when:
Module A exports something from Module B
Module C imports from Module A (getting Module B's export)
Rollup thinks this creates a circular dependency between chunks, even though the actual module dependency is linear

Why this happens:
The warning occurs because Rollup with preserveModules: true sees:
pie-chart.tsx imports from chart-context/index.ts
chart-context/index.ts re-exports from use-chart-registration.ts
Rollup puts these in different chunks and warns about potential execution order issues
However, the actual module resolution is straightforward and linear - there's no circular reference in the JavaScript module graph itself.

Conclusion:
This is a false positive. The code structure is sound and there's no actual circular dependency. This is a known limitation of Rollup when using preserveModules with re-export patterns. The warning can be safely ignored, or you could suppress it by adding a specific warning filter to your Rollup config if it becomes bothersome.

Given it is only a warning and we are exporting all the files and not a bundle I'm inclined to ignore, what do you think @kangzj? We could suppress the warning as it suggests...

I much prefer the imports using indexes, rather than separate imports from each file:

import { useXYChartTheme, useChartDataTransform, useChartMargin, useElementHeight, } from '../../hooks';

Given it is only a warning and we are exporting all the files and not a bundle I'm inclined to ignore, what do you think @kangzj? We could suppress the warning as it suggests...

I much prefer the imports using indexes, rather than separate imports from each file:

Sounds good to me! Happy to address it later as well.

…structure-sharedutilshooks-directories' into charts-86-charts-restructure-sharedutilshooks-directories

kangzj

Thanks for working on this! Much cleaner and clearer 👍

adamwoodnz self-assigned this Aug 27, 2025

github-actions bot added [JS Package] Charts [Status] In Progress RNA labels Aug 27, 2025

github-actions bot added the [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. label Aug 27, 2025

github-actions bot added the [Tests] Includes Tests label Aug 27, 2025

adamwoodnz commented Aug 27, 2025

View reviewed changes

adamwoodnz changed the title ~~Reorganise shared, hooks amd utils~~ Reorganise shared, hooks and utils Aug 27, 2025

adamwoodnz force-pushed the charts-86-charts-restructure-sharedutilshooks-directories branch from d4c2573 to e8c4078 Compare August 27, 2025 22:02

adamwoodnz added [Type] Task and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Aug 27, 2025

adamwoodnz force-pushed the charts-86-charts-restructure-sharedutilshooks-directories branch 2 times, most recently from e8c4078 to 74fc7f2 Compare August 28, 2025 00:06

Base automatically changed from charts-84-charts-stable-colors-for-groups to trunk August 28, 2025 21:17

adamwoodnz added 9 commits August 29, 2025 09:22

Move all hooks and utils out of shared

ed90924

Move remaining shared utils to utils

b230846

Add an index for utils

5b8e38c

Move components out of shared and into their own dirs

d4367a7

Move global chart context hooks into separate files

31648b3

Refactor single chart context exports

f9ac067

Add changelog

cc370de

Rename utils test dir

0c97ade

Rename globals charts things for consistency

7fd6892

adamwoodnz force-pushed the charts-86-charts-restructure-sharedutilshooks-directories branch from 271320e to 7fd6892 Compare August 28, 2025 21:32

adamwoodnz added 2 commits August 29, 2025 09:36

Move chart composition test into dir

33a75f7

Fix import

f1b980e

adamwoodnz added 7 commits August 29, 2025 15:36

Restructure LeaderboardChart

588fc6e

Add a legend private index

678850f

Restructure LineChart

3fce724

Restructure PieChart

7a96fd6

Restructure PieSemiCircleChart

0024898

Add missing type exports to component index files

e833694

Fix some imports from private files

e574c76

adamwoodnz force-pushed the charts-86-charts-restructure-sharedutilshooks-directories branch 2 times, most recently from 36a2b06 to bf9b891 Compare August 29, 2025 04:51

adamwoodnz commented Aug 29, 2025

View reviewed changes

Remove internal types from root exports

2f422a8

adamwoodnz force-pushed the charts-86-charts-restructure-sharedutilshooks-directories branch from bf9b891 to 2f422a8 Compare August 29, 2025 05:00

adamwoodnz added 3 commits August 29, 2025 17:02

Move annotation styles back to component folder

caa255a

Move BaseLegendItem type back to component

87130ad

Fix more type import build warnings

7ad91d6

Revert moving CSS modules to private

f6cf4f1

adamwoodnz force-pushed the charts-86-charts-restructure-sharedutilshooks-directories branch from 1db8c86 to 4b27d18 Compare September 1, 2025 00:19

Organize LineChart types and exports

8a2d7c0

adamwoodnz force-pushed the charts-86-charts-restructure-sharedutilshooks-directories branch from 4b27d18 to 8a2d7c0 Compare September 1, 2025 00:31

adamwoodnz commented Sep 1, 2025

View reviewed changes

Organize LineChart types and exports

8a67369

adamwoodnz force-pushed the charts-86-charts-restructure-sharedutilshooks-directories branch from 8a2d7c0 to 8a67369 Compare September 1, 2025 00:48

kangzj added 3 commits September 1, 2025 12:49

it is actually css for base-legend

8a89ac1

Merge remote-tracking branch 'refs/remotes/origin/charts-86-charts-re…

2189481

…structure-sharedutilshooks-directories' into charts-86-charts-restructure-sharedutilshooks-directories

it is actually css for base-legend

c0a9851

kangzj approved these changes Sep 1, 2025

View reviewed changes

kangzj merged commit 710c54f into trunk Sep 1, 2025
85 checks passed

kangzj deleted the charts-86-charts-restructure-sharedutilshooks-directories branch September 1, 2025 01:14

github-actions bot removed the [Status] Needs Review This PR is ready for review. label Sep 1, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Charts: Reorganise shared, hooks and utils #44971

Charts: Reorganise shared, hooks and utils #44971

Uh oh!

adamwoodnz commented Aug 27, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Aug 27, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Aug 27, 2025 •

edited

Loading

Uh oh!

jp-launch-control bot commented Aug 27, 2025 •

edited

Loading

Uh oh!

adamwoodnz Aug 27, 2025

Uh oh!

adamwoodnz Aug 27, 2025

Uh oh!

adamwoodnz Aug 29, 2025

Uh oh!

adamwoodnz commented Aug 29, 2025 •

edited

Loading

Uh oh!

adamwoodnz Sep 1, 2025 •

edited

Loading

Uh oh!

kangzj Sep 1, 2025

Uh oh!

kangzj left a comment

Uh oh!

Uh oh!

Uh oh!

Charts: Reorganise shared, hooks and utils #44971

Charts: Reorganise shared, hooks and utils #44971

Uh oh!

Conversation

adamwoodnz commented Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes:

Other information:

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

Uh oh!

github-actions bot commented Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jp-launch-control bot commented Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage Summary

Uh oh!

adamwoodnz Aug 27, 2025

Choose a reason for hiding this comment

Uh oh!

adamwoodnz Aug 27, 2025

Choose a reason for hiding this comment

Uh oh!

adamwoodnz Aug 29, 2025

Choose a reason for hiding this comment

Uh oh!

adamwoodnz commented Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adamwoodnz Sep 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kangzj Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

kangzj left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

adamwoodnz commented Aug 27, 2025 •

edited

Loading

github-actions bot commented Aug 27, 2025 •

edited

Loading

github-actions bot commented Aug 27, 2025 •

edited

Loading

jp-launch-control bot commented Aug 27, 2025 •

edited

Loading

adamwoodnz commented Aug 29, 2025 •

edited

Loading

adamwoodnz Sep 1, 2025 •

edited

Loading