fossasia / visdom

A flexible tool for creating, organizing, and sharing visualizations of live, rich data. Supports Torch and Numpy.
Apache License 2.0
9.93k stars 1.13k forks source link

Upgrade from React 16 to React 17 & npm update --save #902

Closed da-h closed 1 year ago

da-h commented 1 year ago

Description

Upgrading from react 16 to react 17 including all dependencies that did not work anymore with react 17. No code changes required.

Note: removing also the package react-select as it is not used in the project anymore.

Motivation and Context

React 17 can be applied safely & directly without any further changes. React 18 on the other hand requires more changes to be made: For this project, this is due to some changes the react-team made to the order of events and renders to be executed internally. This fixes some minor bugs in visdom, that have been expected in some test cases. (The tests did mention the bugs in the code as a comment). With react 18 these bugs are easily fixable. There will be another PR for the React 18 update.

How Has This Been Tested?

-

Note, the javascript-linter is expected to fail here until #900 is merged.

Types of changes

Checklist:

da-h commented 1 year ago

Failing visual tests are false positives. ~Not sure why, but~ some title-texts have moved by less than a pixel after the update and are now rendered with a slightly different aliasing. Edit: Reason is a newer cypress version that renders text slightly differently. To test this, I've fixed the cypress version, leaving the rest of this PR as it is shown here. (See https://github.com/da-h/visdom/pull/20/commits/d478a9f2cb19902ffb7c9dc62762682d71850823). Of course, we can also fix cypress. But I felt that having that one up-to date is the better solution. :)

I've added some of the failing tests as diffs below:

plot_line_opts

plot_line_doubleyaxis

compare_plot_scatter_custom_marker

JackUrb commented 1 year ago

Thanks for digging into the failures here, they're certainly nothing to hold this back!