Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[charts] Issue with imports in DrawingProvider #10178

Closed
2 tasks done
apedroferreira opened this issue Aug 30, 2023 · 4 comments
Closed
2 tasks done

[charts] Issue with imports in DrawingProvider #10178

apedroferreira opened this issue Aug 30, 2023 · 4 comments
Labels
bug 🐛 Something doesn't work component: charts This is the name of the generic UI component, not the React module!

Comments

@apedroferreira
Copy link
Member

apedroferreira commented Aug 30, 2023

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Steps to reproduce 🕹

Steps:

  1. Go to the tools-public app in https://github.com/mui/mui-public
  2. Update the chart component in Toolpad by changing "@mui/toolpad": "^0.1.24" in package.json to "https://pkg.csb.dev/mui/mui-toolpad/commit/e9b0fb8d/@mui/toolpad" - the version from the PR in Migrate Chart component to X charts library toolpad#2500, which implements MUI X charts for the first time
  3. Run app with yarn dev

Current behavior 😯

The app does not load, with the following console error:
Screenshot 2023-08-30 at 18 58 48
Screenshot 2023-08-30 at 18 59 03

Expected behavior 🤔

The app should load with no errors.

Context 🔦

Hopefully this information is helpful in identifying that the issue might be. I guess that it might be an issue with ESM imports like we have had before.

Otherwise let me know and I can try to help in anything that's needed!

That version of Toolpad is using the latest version of the MUI X charts, which should already include the latest fixes for imports, and has the following imports:

import {
  BarPlot,
  LinePlot,
  AreaPlot,
  ScatterPlot,
  MarkPlot,
  BarSeriesType,
  LineSeriesType,
  ScatterSeriesType,
  ScaleName,
} from '@mui/x-charts';
import { ResponsiveChartContainer } from '@mui/x-charts/ResponsiveChartContainer';
import { ChartsXAxis } from '@mui/x-charts/ChartsXAxis';
import { ChartsYAxis } from '@mui/x-charts/ChartsYAxis';
import { ChartsLegend } from '@mui/x-charts/ChartsLegend';
import { ChartsTooltip } from '@mui/x-charts/ChartsTooltip';
import { ChartsAxisHighlight } from '@mui/x-charts/ChartsAxisHighlight';

Will try to confirm as much as possible that this isn't a Toolpad issue just to be sure, sorry if it ends up being that!

Your environment 🌎

Mac, Google Chrome

System:
    OS: macOS 13.4.1
  Binaries:
    Node: 20.3.0 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 9.6.7 - /usr/local/bin/npm
  Browsers:
    Chrome: 116.0.5845.110
    Edge: Not Found
    Safari: 16.5.2
  npmPackages:
    @emotion/react:  11.11.1
    @emotion/styled:  11.11.0
    @mui/base:  5.0.0-beta.12
    @mui/core-downloads-tracker:  5.14.7
    @mui/icons-material:  5.14.6
    @mui/lab:  5.0.0-alpha.141
    @mui/material:  5.14.6
    @mui/private-theming:  5.14.7
    @mui/styled-engine:  5.14.7
    @mui/system:  5.14.6
    @mui/toolpad: https://pkg.csb.dev/mui/mui-toolpad/commit/e9b0fb8d/@mui/toolpad => 0.1.25
    @mui/toolpad-components:  0.1.25
    @mui/toolpad-core:  0.1.25
    @mui/toolpad-utils:  0.1.25
    @mui/types:  7.2.4
    @mui/utils:  5.14.7
    @mui/x-charts:  6.0.0-alpha.8
    @mui/x-data-grid:  6.12.0
    @mui/x-data-grid-pro:  6.12.0
    @mui/x-date-pickers:  6.12.0
    @mui/x-date-pickers-pro:  6.12.0
    @mui/x-license-pro:  6.10.2
    @types/react:  18.2.20
    react:  18.2.0
    react-dom:  18.2.0
    typescript:  5.1.6
    ```
@apedroferreira apedroferreira added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Aug 30, 2023
@zannager zannager added the component: charts This is the name of the generic UI component, not the React module! label Aug 31, 2023
@apedroferreira
Copy link
Member Author

Looks like I fixed it with https://www.npmjs.com/package/@originjs/vite-plugin-commonjs.
Let me know if I should close it, I guess that other people could have the same issue so maybe something can be done from the library side.

@oliviertassinari
Copy link
Member

So related to #9826.

@oliviertassinari oliviertassinari added the bug 🐛 Something doesn't work label Aug 31, 2023
@Janpot
Copy link
Member

Janpot commented Sep 19, 2023

Both this issue and #9826 should be fixed by #10272.

The problem of this issue and #9826 is not that we're not providing a commonjs export. It's that we were declaring in the charts that we have a commonjs export while in reality it didn't work. It didn't work because there is no commonjs version of d3. (And we didn't build a workaround for that limitation yet as we want to spend our time building features first and take this opportunity to probe the community to see how big of a problem it would be to just not build that workaround ever)

Since module resolution amongst bundlers and custom setups these days is basically the wild west, some of them get totally confused. #10272 removes that faulty export so that modern enough bundlers don't get confused. I believe we can close these issues until the community makes a convincing case for us to add a non-broken commonjs version. i.e. Where being on a non-standard setup is justified. While there will be commonjs around for a very long time, the ratio commonjs/ESM will only decrease going forward.

(cc @alexfauquette Would it make sense to create a specific label for issues related to the charts ESM setup? It'd make it easier for me to keep tabs on them)

@alexfauquette alexfauquette added charts: ESM and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Sep 19, 2023
@alexfauquette
Copy link
Member

Created charts: ESM for it 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: charts This is the name of the generic UI component, not the React module!
Projects
None yet
Development

No branches or pull requests

5 participants