Skip to content

fix(echart): Thrown errors shown after resized#33143

Merged
justinpark merged 4 commits intoapache:masterfrom
justinpark:fix--echart-error-swallowed
Apr 17, 2025
Merged

fix(echart): Thrown errors shown after resized#33143
justinpark merged 4 commits intoapache:masterfrom
justinpark:fix--echart-error-swallowed

Conversation

@justinpark
Copy link
Member

@justinpark justinpark commented Apr 16, 2025

SUMMARY

With the introduction of the hotfix related to the #31751 ECharts locale, the init and setOption blocks of ECharts were moved into an async block. This caused an issue where errors occurring during the setOption process were not passed to the react-error-boundary.
To address this issue, this commit separated the locale's async loading process into a separate async block and added a didMount state to ensure that the chart options are properly applied.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:

before--echart-throw-errors.mov

After:

after--echart-throw-errors.mov

TESTING INSTRUCTIONS

Create a sankey chart including a cycle data
The error should be shown after rendering

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@dosubot dosubot bot added the viz:charts:echarts Related to Echarts label Apr 16, 2025
Copy link

@korbit-ai korbit-ai bot left a comment

Choose a reason for hiding this comment

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

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.
Category Issue Status
Logging Unstructured console.error usage ▹ view 🧠 Not in standard
Design Tightly coupled locale loading ▹ view ✅ Fix detected
Files scanned
File Path Reviewed
superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.
Loving Korbit!? Share us on LinkedIn Reddit and X

const lang = await import(`echarts/lib/i18n/lang${locale}`);
return lang?.default;
} catch (e) {
console.error(`Locale ${locale} not supported in ECharts`, e);

This comment was marked as resolved.

Comment on lines 110 to 117
const loadLocale = async (locale: string) => {
try {
const lang = await import(`echarts/lib/i18n/lang${locale}`);
return lang?.default;
} catch (e) {
console.error(`Locale ${locale} not supported in ECharts`, e);
}
};

This comment was marked as resolved.

@justinpark
Copy link
Member Author

cc: @jpchev @pomegranited

@justinpark justinpark merged commit 172e5dd into apache:master Apr 17, 2025
53 checks passed
@michael-s-molina michael-s-molina added the v5.0 Label added by the release manager to track PRs to be included in the 5.0 branch label Apr 24, 2025
michael-s-molina pushed a commit that referenced this pull request Apr 24, 2025
@mistercrunch mistercrunch added 🍒 5.0.0 Cherry-picked to 5.0.0 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels labels Jul 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels plugins size/M v5.0 Label added by the release manager to track PRs to be included in the 5.0 branch viz:charts:echarts Related to Echarts 🍒 5.0.0 Cherry-picked to 5.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants