Skip to content

Make devTools registration opt-in (fixes #6250).#6264

Merged
ckifer merged 2 commits intorecharts:mainfrom
uncaught:issue_6250_dev_tools
Sep 4, 2025
Merged

Make devTools registration opt-in (fixes #6250).#6264
ckifer merged 2 commits intorecharts:mainfrom
uncaught:issue_6250_dev_tools

Conversation

@uncaught
Copy link
Contributor

Description

Makes devTools-registration opt-in via global flag.

Related Issue

#6250

Motivation and Context

The Redux DevTools browser extension does not handle multiple tabs with multiple registered stores well. They are all garbaged together in every instance of the opened dev tools.

You might have a production page open in one tab, showing a few recharts-charts. And then in another tab you have your own development software running, which uses redux for example, but doesn't even have any charts in it. But still you will get all the recharts store entries there.

For recharts developers using this, I would recommend using something like:

window.RECHARTS_DEV_TOOLS_ENABLED = process.env.NODE_ENV === 'development';

How Has This Been Tested?

Used pnpm patch to patch recharts and see the effects in our app in the dev tools.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • I have added a storybook story or VR test, or extended an existing story or VR test to show my changes

This change only affects recharts-developers, not any chart implementation in the user-space.

touchEventMiddleware.middleware,
]),
devTools: {
devTools: !!window.RECHARTS_DEV_TOOLS_ENABLED && {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we enable this by default in storybooks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I've added the flag to the storybook. I hope preview.ts is an appropriate place, it looks like config - I haven't worked with storybooks before.

@codecov
Copy link

codecov bot commented Aug 30, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.61%. Comparing base (9e7fee9) to head (7e76d44).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6264      +/-   ##
==========================================
- Coverage   96.63%   96.61%   -0.03%     
==========================================
  Files         221      221              
  Lines       20147    20147              
  Branches     4129     4130       +1     
==========================================
- Hits        19470    19465       -5     
- Misses        670      675       +5     
  Partials        7        7              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ckifer ckifer merged commit 24dc20b into recharts:main Sep 4, 2025
19 of 20 checks passed
@ckifer
Copy link
Member

ckifer commented Sep 11, 2025

@uncaught this breaks some Next JS builds

.634 ReferenceError: window is not defined
5.634     at b.createRechartsStore (.next/server/chunks/284.js:1:115145)
5.634     at b.RechartsStoreProvider (.next/server/chunks/284.js:1:213462)

touchEventMiddleware.middleware,
]),
devTools: {
devTools: !!window.RECHARTS_DEV_TOOLS_ENABLED && {
Copy link
Member

Choose a reason for hiding this comment

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

we need to check if window is defined

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh dang, please see #6309

I suppose using that Global-module is cleaner than using window anyway.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you 🙌

@uncaught uncaught deleted the issue_6250_dev_tools branch September 12, 2025 05:57
ckifer pushed a commit that referenced this pull request Sep 14, 2025
See #6264

Replaces `window.RECHARTS_DEV_TOOLS_ENABLED` with
`Global.devToolsEnabled`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants