plotly / react-cytoscapejs

React component for Cytoscape.js network visualisations
MIT License
480 stars 68 forks source link

Cytoscape 3.5 compatability #21

Closed brewsoftware closed 5 years ago

brewsoftware commented 5 years ago

Hey, I'm having trouble upgrading to an underlying cytoscape 3.5 version with a Dagre layout. Have the modules changed their API's for registering layouts or is there a reason you are specifically on 3.2.19? Even upgrading to 3.2.20 breaks compatability.

brewsoftware commented 5 years ago

Just confirming, the component works as long as the app is at the exact same version. The reason for the break is that the "extensions" are different when calling cytoscape.use(Layout) vs running the component.

I think this would be solved by decoupling the cytoscape dependency and having it as a peer or you can just publish a new version on the latest cytoscape whenever it's available.

I've internalised the upgrade here but maybe consider moving the version to match the cytoscape version 3.5.x (where x is this component version). vs 1.1 -> 3.19.0 version

vetertann commented 5 years ago

@brewsoftware Hi, could you please explain "internalised the upgrade here" ? Is there workaround to get 3.5.x working for react-cytoscapejs? I'm trying to get dagre extensions to dash-cytoscape, and got here eventually:)

maxkfranz commented 5 years ago

See the following:

$ cd /tmp
$ mkdir foo
$ cd foo/
$ npm init -y
Wrote to /private/tmp/foo/package.json:

{
  "name": "foo",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "keywords": [],
  "author": "",
  "license": "ISC"
}

$ npm i react-cytoscapejs
npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN react-cytoscapejs@1.1.0 requires a peer of react@>=15.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN react-cytoscapejs@1.1.0 requires a peer of react-dom@>=15.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN foo@1.0.0 No description
npm WARN foo@1.0.0 No repository field.

+ react-cytoscapejs@1.1.0
added 9 packages from 14 contributors and audited 9 packages in 2.499s
found 0 vulnerabilities

$ npm i react react-dom
npm WARN foo@1.0.0 No description
npm WARN foo@1.0.0 No repository field.

+ react-dom@16.8.6
+ react@16.8.6
added 3 packages and audited 35 packages in 1.064s
found 0 vulnerabilities

$ npm i cytoscape@3.6.2
npm WARN foo@1.0.0 No description
npm WARN foo@1.0.0 No repository field.

+ cytoscape@3.6.2
updated 1 package and audited 38 packages in 0.63s
found 0 vulnerabilities

$ npm ls cytoscape
foo@1.0.0 /private/tmp/foo
├── cytoscape@3.6.2
└─┬ react-cytoscapejs@1.1.0
  └── cytoscape@3.6.2  deduped

Peer dependencies are for specifying the supported platform version of a plugin:

It makes sense to have react as a peer dependency (this package is basically just a react plugin), but to have cytoscape as a peer dependency would be a bit of an abuse of the npm feature. Other react component plugins follow the same pattern, e.g. @tippyjs/react.

If you are unable to successfully replicate the commands I posted above, let me know.

maxkfranz commented 5 years ago

Your package.json should look like the following:

$ cat package.json
{
  "name": "foo",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "keywords": [],
  "author": "",
  "license": "ISC",
  "dependencies": {
    "cytoscape": "^3.6.2",
    "react": "^16.8.6",
    "react-cytoscapejs": "^1.1.0",
    "react-dom": "^16.8.6"
  }
}
vetertann commented 5 years ago

@maxkfranz thank you, I'll try this

brewsoftware commented 5 years ago

Hey, so yes we did an internal upgrade to 3.5.0 to work with that version and it has been working well. Would prefer to have this done as part of the main package if possible.

TimNZ commented 4 years ago

@maxkfranz agree with commenters that cytoscape should be a peer dependency.

Since the version of cytoscape installed is linked to ^3.2.19 in package.lock/yarn.lock at time of install, adding cytoscape directly later can still result in multiple versions. I had to remove react-cytoscapejs, then delete yarn.lock, and then re-install react-cytoscapejs.