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

Is this an ESM-first module now? #48

Closed
botre opened this issue Sep 14, 2021 · 22 comments · Fixed by #87
Closed

Is this an ESM-first module now? #48

botre opened this issue Sep 14, 2021 · 22 comments · Fixed by #87
Assignees
Labels
bug Something isn't working

Comments

@botre
Copy link

botre commented Sep 14, 2021

When bumping from 10.3.0 to 10.4.4 I encountered the following issue when building my project:

> Build error occurred
Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /home/runner/work/monorepo/monorepo/website/node_modules/d3-array/src/index.js
require() of ES modules is not supported.
require() of /home/runner/work/monorepo/monorepo/website/node_modules/d3-array/src/index.js from /home/runner/work/monorepo/monorepo/website/node_modules/reaviz/dist/index.cjs.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules.
Instead rename index.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from /home/runner/work/monorepo/monorepo/website/node_modules/d3-array/package.json.

Is this an ESM-first module now?

In my opinion, this should be clearly stated in the documentation.

Also, since this breaks backward compatibility (given the current JS ecosystem), I think this should warrant a major version change; not a minor one.

@amcdnl amcdnl self-assigned this Sep 14, 2021
@amcdnl
Copy link
Member

amcdnl commented Sep 14, 2021

@botre - sorry to hear about the issue. let me provide some feedback below.

Is this an ESM-first module now?

It shouldn't be. In the latest release, I upgraded the d3- dependencies which are ESM now, however, these are bundled with the project so they shouldn't be causing this issue.

In my opinion, this should be clearly stated in the documentation.

100% - I did not realize this was happening. If it was I would have documented and bumped major version.

I'm running the project in 2 different CRA applications and not seeing this error. Did you manually include those deps? Can you try deleteing your yarn/npm lock and reinstalling?

@amcdnl amcdnl added the bug Something isn't working label Sep 14, 2021
@botre
Copy link
Author

botre commented Sep 14, 2021

I think CRA supports ESM and transpilation out of the box (unlike Next.js and other tools).
Deleting NPM lock and re-installing did not solve the issue ☹

@botre
Copy link
Author

botre commented Sep 14, 2021

I have created a minimal reproduction for this issue, see this repo: https://github.com/botre/reaviz-next-repro

@amcdnl
Copy link
Member

amcdnl commented Sep 14, 2021

Thank you - Im digging into this - will let you know tomorrow how we can proceed

@botre
Copy link
Author

botre commented Sep 14, 2021

Let me know if there is anything I can help with.

@amcdnl
Copy link
Member

amcdnl commented Sep 17, 2021

OK - I bumped the version to a major so others are aware. I don't know the fix yet though. REAVIZ does have a CJS version but I don't know why the d3-* packages are being difficult. I will keep digging in and let me know if you find anything.

@jazznerd206
Copy link

Hey! Just found this thread, same issue. Has there been a resolution? I am up for a bug hunt if help is required
Screen Shot 2021-11-04 at 10 34 54 AM

@amcdnl
Copy link
Member

amcdnl commented Nov 4, 2021

@jazznerd206 No resolution yet - any help would be greatly appreciated!

@jazznerd206
Copy link

@amcdnl

yeah buddy. i'll give it a shot.

@jazznerd206
Copy link

I don't wanna get too far ahead of myself, but d3 might have solved the problem for us.

The specific dep that's throwing the error for me is d3-array, and according to your package.json, you have v3.0.4, and there is a new version, 3.1.1 available as of 1 month ago. According to the changelog, they have updated it to es6, so import should actually work now.

I'll report back after some testing.

@amcdnl
Copy link
Member

amcdnl commented Nov 8, 2021

Ya - This started when d3-* made their packages ESM and since its a dep of reaviz if your app isn't setup to handle esm then the error happens.

@jazznerd206
Copy link

@amcdnl Hey, so it turns out that was exactly my problem, not a problem with reaviz!!

my fix was as follows:

webpack.config.js
{ test: /\b.json\b/, loader: 'json-loader', },

sorry for conflating my mistake with your package!!

@amcdnl
Copy link
Member

amcdnl commented Nov 9, 2021

Great news - glad you were able to resolve it. Let the group know if you find any of suggestions for this!

@sbsrnt
Copy link

sbsrnt commented Dec 26, 2021

reactviz is also breaking on latest Next (12.0.7) which is supposed to support ESM by default.

In the end managed to get it working with dynamic imports, probably not ideal but at least it's a workaround 🤷🏼

import dynamic from "next/dynamic";

const Component = () => {
  const [CalendarHeatmap, setCalendarHeatmap] = useState<any>(null);
  useEffect(() => {
    // mod.* can be anything you would normally import like BarChart, LineChart etc
    const DynamicCalendarHeatmap = dynamic(() => import('reaviz' as any).then(mod => mod.CalendarHeatmap))

    if(DynamicCalendarHeatmap) {
      setCalendarHeatmap(DynamicCalendarHeatmap)
    }
  }, [])

  return (
    <div>
      {CalendarHeatmap && <CalendarHeatmap data={calendarData} height={300} width="auto"/>}
    </div>
  );
}

"reaviz": "^11.1.2",
"next": "^12.0.7",

Tried to debug the issue but no success on my end either. Here is what I tried:

  1. use next-transpile-modules in next.config.js- still broken
  2. removed cjs from rollup (including commonJs plugin) - still broken
  3. use @jazznerd206 solution - still broken
  4. bumped d3-array to 3.1.1 - still broken
  5. for science went with experimental.esmExternals = true in next.config.js even though it's true by default since 12.x.x (may as well try that at this point lol) - still broken
  6. change rollup-plugin-commonjs to @rollup/plugin-commonjs - still broken
  7. downgraded to next@11.x and tried out with and without experimental.esmExternals = true - still broken
  8. tried urlImports with https://cdn.skypack.dev/reaviz - still broken

At that point the only thing that left besides dynamic import in my mind was to dig in the source maps but didn't bother once I got it working

@amcdnl
Copy link
Member

amcdnl commented Dec 27, 2021

@sbsrnt - Thanks for the detailed report. I'm open to suggestions on ways to address this - I'm not sure what the best path is though.

@dpranav007
Copy link

@amcdnl Could you please check this for the issue. Thanks
vercel/next.js#33901 (comment)

@amcdnl
Copy link
Member

amcdnl commented Apr 5, 2022

@dpranav007 - Thanks for the ping. The comment in that ticket is very helpful. I need to research an alternative build configurations and in the meantime if u find anything relevant post in this thread.

@zischwartz
Copy link

In the end managed to get it working with dynamic imports, probably not ideal but at least it's a workaround 🤷🏼

This is still an issue for with next version 12.1.0 (and reavis 13.0.4) but I've got a much simpler work around for anyone coming across this:

import dynamic from "next/dynamic";
const BarChart = dynamic(() => import("reaviz").then((mod) => mod.BarChart), {
  ssr: false,
});

Next docs. The SSR :false option seems to fix this issue (at the obvious cost of not rendering on the server)

@HarryTranCustodio
Copy link

HarryTranCustodio commented May 12, 2022

Hi I've tried using @zischwartz solution but it's not working when I try to import LinearXAxis and LinearYAxis. Below is my code.

const BarChart = dynamic(() => import('reaviz').then(mod => mod.BarChart), {ssr: false});
const BarSeries = dynamic(() => import('reaviz').then(mod => mod.BarSeries), {ssr: false});
const LinearXAxis = dynamic(() => import('reaviz').then(mod => mod.LinearXAxis), {ssr: false});
const LinearYAxis = dynamic(() => import('reaviz').then(mod => mod.LinearYAxis), {ssr: false});

const data = [
    {key: "PowerShell - T1059", data: 150},
    {key: "Remote Desktop Protocol - T1021", data: 200},
    {key: "Data Encrypted for Impact - T1486", data: 300}
]

export default function MitreBarChart() {    
    return (
        <BarChart
            data={data}
            xAxis={<LinearXAxis type="value"/>}
            yAxis={<LinearYAxis type="category"/>}
            series={<BarSeries layout="horizontal"/>}
        />
    )
}

This is the error:

image

As we can see from the call stack, the error is coming from the isAxisVisible function, which is trying to read the props from LinearXAxis or LinearYAxis. I can't figure out why they are undefined.

Can anyone advise on how to fix this?

@amcdnl
Copy link
Member

amcdnl commented May 12, 2022

I'm happy to implement any suggestion someone might have, I just don't know the best way to fix this. I've also not hit it but I'm not doing NEXT/etc.

@jleei
Copy link

jleei commented Sep 17, 2022

same issue, not work in cra with ssr:

module.exports = require("d3-time-format");

Error [ERR_REQUIRE_ESM]: require() of ES Module .../node_modules/d3-time-format/src/index.js from .../build/server.js not supported.
[1] Instead change the require of index.js in .../build/server.js to a dynamic import() which is available in all CommonJS modules.

@amcdnl
Copy link
Member

amcdnl commented Oct 11, 2022

Please give this a try again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants