Update react/react-dom to 16.13.0 and docgen to 5.3.0 [ref dash#1531] #117

AnnMarieW · 2021-01-18T21:48:38Z

This pull request is for updating the docstring comments so they will be compatible with the new style docstrings in dash #1531

This project started with plotly/dash#1205

You can see results for other libraries in dash-doc #https://github.com/plotly/dash-docs/pull/1063

This repo will need to be rebuilt with a more recent version of docgen than the one currently specified (2.21.0). I rebuilt the library with docgen 5.3.0, react 16.13.1 and react-dom 16.13.1. I see that #81 is still open, so I don't know if these version changes will cause any problems. The rebuilt library passed all tests.

Here are some before and after screenshots for dash-cytoscape in dash-docs and in the python console.

Before:

After

In the console: Before:

In the console: After:

xhluca · 2021-01-18T23:39:20Z

@AnnMarieW Thank you for opening the PR, the updated docstring looks much better! And thank you for the thorough and well-written PR description.

@alexcjohnson @Marc-Andre-Rivet I noticed that the dash version in requirements.txt is fairly outdated; do you think this could affect the build process wrt the react and react-dom version?

AnnMarieW · 2021-01-19T00:41:52Z

@xhlulu I'm glad you like it! I forgot to mention that I also used the latest version of dash to rebuild and run the tests. The main issue I ran into was that docgen 2.21.0 didn't recognized PropTypes.exact so it needed to be upgraded. I just bumped it to a version being used in dash-table

alexcjohnson · 2021-01-19T14:24:08Z

hah yes, 0.39 is stretching the definition of "fairly" outdated at this point. But what is requirements.txt even used for? setup.py only lists dash (and with no version). There's also tests/requirements.txt listing an extremely old dash (but with >= so it should be getting the latest) as well as pytest-dash - would be great at some point to transition this to dash[testing] but that will require rewriting some of the tests themselves.

xhluca · 2021-01-20T18:56:05Z

@alexcjohnson Thanks for mentioning this. requirements.txt would ideally be the requirements needed to run the demos locally and developing the libraries. Perhaps it's time to delete test/requirements.txt and move everything to requirements.txt, so we won't have to track/manage two requirements files?

I've also created a project to track everything relating to tests, as well as opened an issue to track the refactoring of tests.

xhluca

I've added the build here in this test branch: https://github.com/plotly/dash-cytoscape/tree/update-docgen

I don't really have anything to say if the tests pass. @alexcjohnson is there anything else before 💃 ?

alexcjohnson · 2021-01-20T19:15:05Z

This looks great to me. @AnnMarieW let's get https://github.com/plotly/dash-docs/pull/1063 merged (WITHOUT any updates to the component packages) before we merge any of the updates to the component packages themselves.

AnnMarieW · 2021-01-20T19:18:47Z

Sounds good. And plotly/dash#1531 will also need to be merged before updating the component packages?

src/lib/components/Cytoscape.react.js

xhluca

@AnnMarieW Sorry for getting back to you so late, I didn't realize dash#1531 was merged! I suggested a few changes and I'll merge it once I figure out what's wrong with the percy tests!

src/lib/components/Cytoscape.react.js

xhluca · 2021-02-11T17:10:14Z

package.json

@@ -40,9 +40,9 @@
    "cytoscape-svg": "0.2.0",
    "lodash": "^4.17.11",
    "ramda": "^0.25.0",
-    "react": "15.4.2",
+    "react": "^16.13.1",


I wonder if it's better to fix it to 16.13.0 since that's what's done on plotly/dash-core-components. Not sure if it might break dash if react/react-dom are upgraded beyond that.

Suggested change

"react": "^16.13.1",

"react": "16.13.0",

xhluca · 2021-02-11T17:10:37Z

package.json

    "react-cytoscapejs": "1.2.1",
-    "react-dom": "15.4.2"
+    "react-dom": "^16.13.1"


Same as above

Suggested change

"react-dom": "^16.13.1"

"react-dom": "16.13.0"

xhluca · 2021-02-11T17:12:27Z

package.json

@@ -57,7 +57,7 @@
    "eslint-plugin-import": "^2.12.0",
    "eslint-plugin-react": "^7.9.1",
    "npm": "^6.14.4",
-    "react-docgen": "^2.21.0",
+    "react-docgen": "^5.3.0",


Wonder if a ~ would be better than ^ here (I just learned the differences here)

Co-authored-by: Xing Han Lu <xhlu+github@pm.me>

AnnMarieW · 2021-02-11T18:14:18Z

@xhlulu Thanks for the suggested changes. I approved the changes from exact to shape and will leave the rest open for you or the other reviewers.

xhluca · 2021-02-11T18:29:15Z

@AnnMarieW Thank you!

@alexcjohnson I'm happy to merge it as is and I can figure out the right package.json dependencies and building in another PR

xhluca

💃

alexcjohnson

💃

Many warnings are caused by incorrect proptypes introduced in #117

* npm run build with dash 1.20 * Bump ssri from 6.0.1 to 6.0.2 (#131) Bumps [ssri](https://github.com/npm/ssri) from 6.0.1 to 6.0.2. - [Release notes](https://github.com/npm/ssri/releases) - [Changelog](https://github.com/npm/ssri/blob/v6.0.2/CHANGELOG.md) - [Commits](npm/ssri@v6.0.1...v6.0.2) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump ini from 1.3.5 to 1.3.8 (#115) Bumps [ini](https://github.com/isaacs/ini) from 1.3.5 to 1.3.8. - [Release notes](https://github.com/isaacs/ini/releases) - [Commits](npm/ini@v1.3.5...v1.3.8) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump dot-prop from 4.2.0 to 4.2.1 (#124) Bumps [dot-prop](https://github.com/sindresorhus/dot-prop) from 4.2.0 to 4.2.1. - [Release notes](https://github.com/sindresorhus/dot-prop/releases) - [Commits](sindresorhus/dot-prop@v4.2.0...v4.2.1) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * npm audit fix * Update changelog * Fix prop types to stop warnings Many warnings are caused by incorrect proptypes introduced in #117 * npm run build Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

update docstrings to be compantible with dash#1531

ee1b0b6

AnnMarieW requested review from alexcjohnson, Marc-Andre-Rivet and xhluca as code owners January 18, 2021 21:48

AnnMarieW changed the title ~~update docstrings to be compantible with dash#1531~~ update docstrings to be compatible with dash#1531 Jan 18, 2021

xhluca mentioned this pull request Jan 20, 2021

Rewrite the tests #118

Closed

xhluca reviewed Jan 20, 2021

View reviewed changes

xhluca reviewed Jan 21, 2021

View reviewed changes

src/lib/components/Cytoscape.react.js Outdated Show resolved Hide resolved

xhluca mentioned this pull request Feb 11, 2021

Update Dash to 1.* and React to 16.13 #122

Closed

6 tasks

xhluca changed the title ~~update docstrings to be compatible with dash#1531~~ Update react/react-dom to 16.13.0 and docgen to 5.3.0 [ref dash#1531] Feb 11, 2021

xhluca suggested changes Feb 11, 2021

View reviewed changes

AnnMarieW and others added 3 commits February 11, 2021 11:09

Update src/lib/components/Cytoscape.react.js

7fa668d

Co-authored-by: Xing Han Lu <xhlu+github@pm.me>

Update src/lib/components/Cytoscape.react.js

c785c1d

Co-authored-by: Xing Han Lu <xhlu+github@pm.me>

Update src/lib/components/Cytoscape.react.js

907fd3c

Co-authored-by: Xing Han Lu <xhlu+github@pm.me>

xhluca approved these changes Feb 11, 2021

View reviewed changes

alexcjohnson approved these changes Feb 11, 2021

View reviewed changes

xhluca merged commit cde1e4b into plotly:master Feb 11, 2021

AnnMarieW deleted the update-docstrings branch February 12, 2021 00:19

xhluca pushed a commit that referenced this pull request Feb 12, 2021

Update changelog based on #117 and #123

e3493b5

xhluca mentioned this pull request Feb 12, 2021

React 16.12 #81

Closed

xhluca pushed a commit that referenced this pull request Apr 30, 2021

Fix prop types to stop warnings

f2c4519

Many warnings are caused by incorrect proptypes introduced in #117

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update react/react-dom to 16.13.0 and docgen to 5.3.0 [ref dash#1531] #117

Update react/react-dom to 16.13.0 and docgen to 5.3.0 [ref dash#1531] #117

AnnMarieW commented Jan 18, 2021

xhluca commented Jan 18, 2021

AnnMarieW commented Jan 19, 2021

alexcjohnson commented Jan 19, 2021

xhluca commented Jan 20, 2021

xhluca left a comment

alexcjohnson commented Jan 20, 2021

AnnMarieW commented Jan 20, 2021

xhluca left a comment

xhluca Feb 11, 2021

xhluca Feb 11, 2021

xhluca Feb 11, 2021

AnnMarieW commented Feb 11, 2021

xhluca commented Feb 11, 2021

xhluca left a comment

alexcjohnson left a comment

Update react/react-dom to 16.13.0 and docgen to 5.3.0 [ref dash#1531] #117

Update react/react-dom to 16.13.0 and docgen to 5.3.0 [ref dash#1531] #117

Conversation

AnnMarieW commented Jan 18, 2021

Before:

In the console: Before:

In the console: After:

xhluca commented Jan 18, 2021

AnnMarieW commented Jan 19, 2021

alexcjohnson commented Jan 19, 2021

xhluca commented Jan 20, 2021

xhluca left a comment

Choose a reason for hiding this comment

alexcjohnson commented Jan 20, 2021

AnnMarieW commented Jan 20, 2021

xhluca left a comment

Choose a reason for hiding this comment

xhluca Feb 11, 2021

Choose a reason for hiding this comment

xhluca Feb 11, 2021

Choose a reason for hiding this comment

xhluca Feb 11, 2021

Choose a reason for hiding this comment

AnnMarieW commented Feb 11, 2021

xhluca commented Feb 11, 2021

xhluca left a comment

Choose a reason for hiding this comment

alexcjohnson left a comment

Choose a reason for hiding this comment