Error: Cannot find module '@nivo/core' after upgrading to 0.65.0

plouc / nivo

nivo provides a rich set of dataviz components, built on top of the awesome d3 and React libraries

https://nivo.rocks

MIT License

13.19k stars 1.03k forks source link

Error: Cannot find module '@nivo/core' after upgrading to 0.65.0 #1286

Closed p45mark closed 3 years ago

p45mark commented 3 years ago

Describe/explain the bug After upgrading the Pie and Line charts to version 0.65.0 I'm getting the error Module not found: Can't resolve '@nivo/core'

To Reproduce In this codesandbox just bump the version of @nivo/pie to 0.65.0 to see the error https://codesandbox.io/s/nivo-core-required-zk09f?file=/src/App.js

Steps to reproduce the behavior:

Go here: https://codesandbox.io/s/nivo-core-required-zk09f?file=/src/App.js
Bump version to 0.65.0

Desktop (please complete the following information):

OS: Mac Catalina
Chrome

Additional context Maybe we just need to update the documentation if @nivo/core is required now

orhels commented 3 years ago

Had the same issue. Added @nivo/core as a dependency to fix it.

wyze commented 3 years ago

It is a peer dependency on all of the packages to prevent cyclical dependencies between core and tooltip. So like orhels mentions, you need to specify it in your dependencies now.

wyze commented 3 years ago

Yes, updating the documentation/readme would be a step in the right direction. :)

orhels commented 3 years ago

@wyze Note that the peer dependency is set to 0.64.0 for some of the packages. Checked the ones I use (annotations, axes, bar, colors, legends, pie and tooltip) and all has 0.64.0 as a peer dependency. When testing pie with core 0.64.0 in code sandbox the app crashes https://codesandbox.io/s/nivo-core-required-forked-gdd1x.

wyze commented 3 years ago

Yup, we need to fix that with lerna, it isn't bumping peer dependencies correctly.

plouc commented 3 years ago

It's been fixed with 0.65.1 for charts, which now point to @nivo/core@0.65.0.

plouc commented 3 years ago

We also added some install instructions in the generated code snippet on the website.

mwskwong commented 3 years ago

So just to make sure I get it right, we now have to explicitly specify @nivo/core@^0.65.0 in package.json for v0.65.x (perhaps also in future versions)?

wyze commented 3 years ago

@matthewkwong2 Correct. You should bump @nivo/core dependency when you bump your other packages as well.

mwskwong commented 3 years ago

Well, I think you should really mention this in the release note (or maybe make it more obvious if it is mentioned somewhere). This is a breaking change.

plouc commented 3 years ago

@matthewkwong2, it is now indicated in the generated code snippet, also when you install, you'll get warnings about required versions.

plouc commented 3 years ago

This was already the case for a few package before the latest release, but the main difference is that the version defined in peerDependencies is now correct.

arthuryeti commented 3 years ago

@plouc are you sure ? Because I'm doing this, but it doesn't seems to work, because peerDependencies are still not on the same level.

    "@nivo/core": "0.65.1",
    "@nivo/bar": "0.65.1",
    "@nivo/funnel": "0.65.1",
    "@nivo/pie": "0.65.1",
    "@nivo/sankey": "0.65.1",
    "@nivo/stream": "0.65.1",

wyze commented 3 years ago

@arthuryeti In this case, there were no changes detected in core so it is still on 0.65.0 which is the same version the peerDependency is in the chart packages.

arthuryeti commented 3 years ago

@wyze I think the core should be updated, because right now it depends on @nivo/tooltip@0.63.0 which break the npm install

plouc commented 3 years ago

@arthuryeti, you're right, we didn't consider this other peer dependency.

plouc commented 3 years ago

We might want to make the script a bit more generic @wyze.

wyze commented 3 years ago

Done in #1293.