carbon-design-system / carbon-charts

:bar_chart: :chart_with_upwards_trend:⠀Robust dataviz framework implemented using D3 & typescript
https://charts.carbondesignsystem.com
Apache License 2.0
904 stars 184 forks source link

[Bug]: charts-react SSR render fails starting with 1.13.10 #1856

Closed David-Yuen closed 3 months ago

David-Yuen commented 3 months ago

Relevant package(s)

Carbon Charts Version

1.13.10 to latest

Which bundler are you using?

Webpack

What happened and what did you expect to happen?

Our application uses server side rendering of our pages, and starting with 1.13.10, the page fails to render. The last working build is "@carbon/charts-react": "1.13.8",. (I tried 1.13.9 but npm could not find it, doesnt seem to exist anymore).. Starting with 1.13.10, when we render our page in the browser, the page fails to render, and on the server side terminal i see this error:

Error: Failed to render app on server with exception Cannot find module '/Users/davidyuen/git/lego-ui/node_modules/@carbon/charts-react/dist/index.js' Error: Cannot find module '/Users/davidyuen/git/lego-ui/node_modules/@carbon/charts-react/dist/index.js'
    at createEsmNotFoundErr (node:internal/modules/cjs/loader:1182:15)
    at finalizeEsmResolution (node:internal/modules/cjs/loader:1170:15)
    at resolveExports (node:internal/modules/cjs/loader:592:14)
    at Function.Module._findPath (node:internal/modules/cjs/loader:669:31)
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:1131:27)
    at Function.Module._load (node:internal/modules/cjs/loader:986:27)
    at Module.require (node:internal/modules/cjs/loader:1233:19)
    at require (node:internal/modules/helpers:179:18)
    at Object.@carbon/charts-react (/Users/davidyuen/git/lego-ui/build/node/replication-details.js:22275:18)
    at __webpack_require__ (/Users/davidyuen/git/lego-ui/build/node/replication-details.js:22629:41)
    at Object../src/components/ReplicationDetails/ReplicationDetailsMetrics/LineGraph/LineGraph.js (/Users/davidyuen/git/lego-ui/build/node/replication-details.js:7378:78)

On the latest build 1.16.9, the server side message is slightly different showing:

[2024-06-17T09:45:43.933] [ERROR] Failed to render page: /mappings/6a8c4bb3-11d5-408d-8019-b5467bda5393/details?project_id=d54f45c6-47e1-4530-803e-5f4acc946a3f&context=cpdaas
Error: Failed to render app on server with exception require() of ES Module /Users/davidyuen/git/lego-ui/node_modules/@carbon/charts-react/dist/index.js from /Users/davidyuen/git/lego-ui/build/node/replication-details.js not supported.
index.js is treated as an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which declares all .js files in that package scope as ES modules.
Instead either rename index.js to end in .cjs, change the requiring code to use dynamic import() which is available in all CommonJS modules, or change "type": "module" to "type": "commonjs" in /Users/davidyuen/git/lego-ui/node_modules/@carbon/charts-react/package.json to treat all .js files as CommonJS (using .mjs for all ES modules instead).
 Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/davidyuen/git/lego-ui/node_modules/@carbon/charts-react/dist/index.js from /Users/davidyuen/git/lego-ui/build/node/replication-details.js not supported.
index.js is treated as an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which declares all .js files in that package scope as ES modules.
Instead either rename index.js to end in .cjs, change the requiring code to use dynamic import() which is available in all CommonJS modules, or change "type": "module" to "type": "commonjs" in /Users/davidyuen/git/lego-ui/node_modules/@carbon/charts-react/package.json to treat all .js files as CommonJS (using .mjs for all ES modules instead).

    at @carbon/charts-react (/Users/davidyuen/git/lego-ui/build/node/replication-details.js:22275:18)
    at __webpack_require__ (/Users/davidyuen/git/lego-ui/build/node/replication-details.js:22629:41)

I'm not sure what changed, but starting with patch updates, something broke between 1.13.8 and 1.13.10, and this issue is related to importing chart components. Seems like something changed and on SSR side cannot import chart components properly with import { LineChart } from "@carbon/charts-react";.

More details: Code use to work in carbon v10, and I am currently updating to carbon v11.. in carbon charts v11, things worked until 1.13.8 but breaks after 1.13.10.

Since we use server side rendering.. normally is after i install all packages (npm install), we build our application with npm run build (which builds both server and client side), the build succeeds. Then I start up the server with npm start, while in local dev i have hot-module replacement so it rebuilds automatically in-memory for client-side code. Both starting up and re-building succeeds. Then i load a page with charts on it, and the page fails to render. On server side I can see server-side rendering errors (above).

If i comment out the import charts and render charts lines.. like:

//import { LineChart } from "@carbon/charts-react";

...

render() {
  return (
    //<LineChart .. />
  );
}

I re-run the build for both server and client (npm run build), start the server, and then load the page, everything loads and renders fine. Then, since i am in dev mode, I can un-comment those lines, the code will auto-rebuild (this is in-memory and for client side only), and i reload the page.. The charts will load and render fine as well.

I have also asked in carbon-charts channel for help, I also got an example using DonutCharts and same error occurs..

So I've narrowed the problem down to only on server side, there are issues with importing charts components.

Chart data and options (automatically formatted so no need for backticks)

//Error occurs with all charts, an issue related with importing chart components...
//But to clarify.. my ReactJS file has something like this:

import { LineChart } from "@carbon/charts-react";

...

render() {
  return (
    <LineChart .. />
  );
}

//This error happens on server side regardless of which chart and data used. I asked for help in carbon-charts channel and I got a sample chart that also reproduced the issue.

JavaScript console or build output (if relevant)

//1.13.10
Error: Failed to render app on server with exception Cannot find module '/Users/davidyuen/git/lego-ui/node_modules/@carbon/charts-react/dist/index.js' Error: Cannot find module '/Users/davidyuen/git/lego-ui/node_modules/@carbon/charts-react/dist/index.js'
    at createEsmNotFoundErr (node:internal/modules/cjs/loader:1182:15)
    at finalizeEsmResolution (node:internal/modules/cjs/loader:1170:15)
    at resolveExports (node:internal/modules/cjs/loader:592:14)
    at Function.Module._findPath (node:internal/modules/cjs/loader:669:31)
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:1131:27)
    at Function.Module._load (node:internal/modules/cjs/loader:986:27)
    at Module.require (node:internal/modules/cjs/loader:1233:19)
    at require (node:internal/modules/helpers:179:18)
    at Object.@carbon/charts-react (/Users/davidyuen/git/lego-ui/build/node/replication-details.js:22275:18)
    at __webpack_require__ (/Users/davidyuen/git/lego-ui/build/node/replication-details.js:22629:41)
    at Object../src/components/ReplicationDetails/ReplicationDetailsMetrics/LineGraph/LineGraph.js (/Users/davidyuen/git/lego-ui/build/node/replication-details.js:7378:78)

//1.16.9
[2024-06-17T09:45:43.933] [ERROR] Failed to render page: /mappings/6a8c4bb3-11d5-408d-8019-b5467bda5393/details?project_id=d54f45c6-47e1-4530-803e-5f4acc946a3f&context=cpdaas
Error: Failed to render app on server with exception require() of ES Module /Users/davidyuen/git/lego-ui/node_modules/@carbon/charts-react/dist/index.js from /Users/davidyuen/git/lego-ui/build/node/replication-details.js not supported.
index.js is treated as an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which declares all .js files in that package scope as ES modules.
Instead either rename index.js to end in .cjs, change the requiring code to use dynamic import() which is available in all CommonJS modules, or change "type": "module" to "type": "commonjs" in /Users/davidyuen/git/lego-ui/node_modules/@carbon/charts-react/package.json to treat all .js files as CommonJS (using .mjs for all ES modules instead).
 Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/davidyuen/git/lego-ui/node_modules/@carbon/charts-react/dist/index.js from /Users/davidyuen/git/lego-ui/build/node/replication-details.js not supported.
index.js is treated as an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which declares all .js files in that package scope as ES modules.
Instead either rename index.js to end in .cjs, change the requiring code to use dynamic import() which is available in all CommonJS modules, or change "type": "module" to "type": "commonjs" in /Users/davidyuen/git/lego-ui/node_modules/@carbon/charts-react/package.json to treat all .js files as CommonJS (using .mjs for all ES modules instead).

    at @carbon/charts-react (/Users/davidyuen/git/lego-ui/build/node/replication-details.js:22275:18)
    at __webpack_require__ (/Users/davidyuen/git/lego-ui/build/node/replication-details.js:22629:41)

StackBlitz repro

No response

IBM Application/Team (if relevant)

No response

What priority level would this be in your opinion?

P1 (High)

nstuyvesant commented 3 months ago

Hi @David-Yuen - thanks for this detail. Let's figure out how to get you working with 1.16.9 (and beyond).

The way I'm reading the most recent error is that you're requiring node_modules/@carbon/charts-react/dist/index.js and getting a message that it's an ES module and not UMD. But if we look at that file, we can visually see it's UMD because the file starts out with an IIFE.

It's being treated as an ES module because @carbon/charts-react package.json's type is module. This treats .js files as ES modules even though that's not correct where the dist folder is concerned.

Can you try a quick experiment?

  1. In your node_modules/@carbon/charts-react/package.json, remove line 5 ("type": "module") and save.
  2. See if the error still occurs.

Also, can you share what version of webpack you are using as well as your webpack config?

Separate question, are you able to use import instead of require?

nstuyvesant commented 3 months ago

@David-Yuen - we changed the exports for the UMD to use CJS in 1.16.10. Can you please check that and see if it solves your issue?

David-Yuen commented 3 months ago

@nstuyvesant I just tested 1.16.10 and it looks like the issue is resolved now, thanks.

I was going to try your quick experiment but you guys already fixed and delivered the 1.16.10, and that's working, so i think i dont need to test this now..

and answering the other questions:

We are using "webpack": "5.92.0", our webpack config is a bit complex, if you send me a slack message i can share it with you. We have one webpack config that builds client code in both node and web versions. We generally followed DAP's teams patterns.

and we generally use import from all our reactjs files.. but after building, the files are compiled differently and not sure how it is actually executed.. We are using the @loadable/component and @loadable/server libraries for code splitting, so we have built routes to each of our SPAs. Our webpack config will build each of our SPAs for both node and web.. I just generally know the node and web compiled codes are quite different.

nstuyvesant commented 3 months ago

Hi @David-Yuen - I am not inside of IBM so not on Slack but glad the update solved your issue.

Something that I have seen among Webpack users is inadvertent use of a cjs-loader. Normally, this is to convert ES modules to CJS. But if you wrote everything using import, you may not need it.

For sure, your generated bundles are CJS. You could look into whether that's really required. The benefit for going pure ESM is better tree-shaking. Of course, you are doing SSR so you would need to be using a recent version of node.js that supports ESM.

In any case, glad this blocker has been overcome. All the best.