Update toolchain #939

Marc-Andre-Rivet · 2021-03-24T15:51:00Z

Depends on plotly/dash#1563
Fixes partially https://github.com/plotly/dash-core/issues/203

Related to plotly/dash-table#878

Update CircleCI images to newest available + bump py37 images to py39
Rewrites legacy tests with the dash_dcc fixture (except for test_dash_import which is just a pure unit test)
Add dash_dcc.get_logs() assert to tests that don't have one (uncovered 5 tests that were broken or at least badly configured by adding that check)
Proposes to 🔪 the Jest tests (the carrying cost for these has been high on every non-trivial toolchain update and the benefit they bring isn't clear.. I will review the tests content and see if some should be rewritten as integration tests.. more to come)
General toolchain updates
Post merge need to remove ci/circleci: test-legacy-xx, ci/circleci: test-37, and ci/circleci: lint-unit-37 checks

TODO (ACJ 2021-07-07): five percy snapshots still have problems:

styled input - width: 100%, border-color: hotpink -> missing styles?
mkdw002 - markdowns display -> missing images, title_crumb shows up as link text
mkdw001 - image display -> no image (should there only be one as in baseline? or two?)
slsl005- dcc.RangeSlider tooltip position -> bad tooltip positions
dtpr005 - disabled days -> first chosen day changed color

… files // gut out files that serve no purpose

Marc-Andre-Rivet · 2021-03-24T15:51:38Z

.circleci/config.yml

-            find . -name "dash_*.gz" | xargs pip install -I --progress-bar off --no-cache-dir
-            pip install --no-cache-dir --progress-bar off main.tar.gz[dev,testing]
-            pip list | grep dash | xargs pip show && cd ..
-            echo $(python -V 2>&1) | grep 3. && python -m unittest tests/test_dash_import.py


Only test_dash_import remains and is moved here https://github.com/plotly/dash-core-components/pull/939/files#diff-78a8a19706dbd2a4425dd72bdab0502ed7a2cef16365ab7030a5a0588927bf47R158

Marc-Andre-Rivet · 2021-03-24T15:52:25Z

.flake8

@@ -1,5 +1,5 @@
 [flake8]
-ignore = C901, E203, E266, E501, E731, W503
+ignore = C901, E203, E231, E266, E501, E731, W503


Black/flake8 disagreement about spaces after trailing comma. Black wins over Flake8, same as in the dash-table.

Marc-Andre-Rivet · 2021-03-24T15:53:17Z

MANIFEST.in

-include dash_core_components/async-*.js
-include dash_core_components/async-*.js.map
+include dash_core_components/*.js
+include dash_core_components/*.js.map


This is consistent with what is being done in other repos and how these deps are published in the npm package.

Marc-Andre-Rivet · 2021-03-24T15:54:06Z

dash_core_components_base/__init__.py

@@ -76,18 +76,18 @@
 _js_dist.extend(
    [
        {
-            "relative_package_path": "{}.min.js".format(__name__),
+            "relative_package_path": "{}.js".format(__name__),


There hasn't been a dev vs. prod build of this package since we've moved prop-types from the component packages to the dash-renderer.

Marc-Andre-Rivet · 2021-03-24T15:55:49Z

package.json

-    "file-saver": "^2.0.2",
-    "highlight.js": "^10.3.1",
-    "moment": "^2.20.1",
+    "base64-js": "^1.5.1",


Update everything to latest except for react-dates and react-dropzone major bumps that cause props and visual changes. Bumping those should be part of a future general Dash 2.x discussion

Dunno if we have appetite for this in Dash 2, but we should at least discuss.

Marc-Andre-Rivet · 2021-03-24T15:56:14Z

package.json

  },
  "devDependencies": {
-    "@babel/cli": "^7.4.0",
-    "@babel/core": "^7.4.0",
-    "@babel/plugin-proposal-nullish-coalescing-operator": "^7.10.4",


These are redundant with recent versions of babel

Marc-Andre-Rivet · 2021-03-24T15:56:31Z

package.json

-    "component-playground": "^3.0.0",
-    "copyfiles": "^2.0.0",
-    "css-loader": "^1.0.1",
-    "enzyme": "^3.7.0",


Enzyme and Jest 🔪

Marc-Andre-Rivet · 2021-03-24T15:57:23Z

tests/integration/dropdown/test_option_title_prop.py

@@ -2,12 +2,11 @@

 import pytest
 import dash
+from dash.testing import wait


Updated to use the common wait function instead of the dcc home-brewed one

matches what we're doing in dash - 27, 36, 39

and update react-markdown to latest in v4

alexcjohnson · 2021-07-08T00:17:44Z

package.json

+    "prettier": "^2.3.2",
+    "react": "^16.14.0",
+    "react-dom": "^16.14.0",
+    "react-jsx-parser": "1.21.0",


Pinned - upgrading react-jsx-parser to 1.28 breaks dcc.Markdown with html allowed.

alexcjohnson · 2021-07-08T01:16:01Z

tests/integration/sliders/test_sliders.py

@@ -159,7 +159,7 @@ def test_slsl005_slider_tooltip(dash_dcc):
                        style=dict(height=100),
                    ),
                ],
-                style=dict(maxHeight=300, overflowX="scroll"),
+                style=dict(maxHeight=300, overflowX="scroll", width=400),


Slider tooltips don't render correctly on Percy. They do pretty well when the width is locked like this, though there's still a bit of an offset. But when the width is set by the page they're way off: Most of the slider, and its handles, are positioned in percentages but the tooltips are positioned in pixels. Ironic, because the tooltips and handles could have used the same positioning logic: left: <some %>; transform: translateX(-50%). But when Percy picks this up it rerenders the page with a different width and pixels and % no longer have the same relationship at all.

I suspect there's a real bug here, perhaps if your slider is subject to CSS scaling or if you change its width through a style prop after rendering; but I haven't tested it. Sliders do update correctly when their width updates as the window is resized though, which is the most important use case.

.circleci/config.yml

eff-kay · 2021-07-08T06:01:02Z

tests/integration/calendar/test_calendar_props.py

@@ -42,6 +42,8 @@ def test_cdpr001_date_clearable_true_works(dash_dcc):
    (single_date,) = dash_dcc.get_date_range("dps")
    assert not single_date, "date should be cleared"

+    assert dash_dcc.get_logs() == []


helpful message on assertion failure.

This is a pattern we use at the end of most tests - it just means "there are no errors in the JS console"

We could probably make it clearer to read though by building this directly into the fixture, like:

dash_duo.assert_clean_console()

(dash_dcc is a dash_duo clone with some dcc-specific additions. I say clone because it behaves like a subclass but isn't really... the subclassing happens one layer down.)

webpack.config.js

eff-kay · 2021-07-08T18:34:27Z

LGTM

Marc-Andre-Rivet added 22 commits March 22, 2021 16:47

update toolchain + delete jest tests

727a2ab

remove unit tests from CI

f9473df

remove .min

0a996ef

no cache

e8d45f1

add back cache / change name

05c5531

simplify manifest

2f8253e

bust cache again

b9e1e5b

undo filename

39b1021

node polyfills

1a7f98a

update loaders / insertAt equivalent logic

ca2388b

update react-resize-detector

670c60a

eslint re-upgrade

fd06f77

update ci images + py39

5526971

remove .min. from the pacakge's main entrypoint

6c37bef

rewrite test_integration_1 tests with fixture

8b96292

lint

aa99547

rewrite legacy tests with dash_dcc // gut out additional jest testing…

8a8a946

… files // gut out files that serve no purpose

fix linting

86c5a05

fix new tests

e9c5016

black

7e52059

Add back test_dash_import

6609e7c

clean up cache busting

3442c2e

Marc-Andre-Rivet commented Mar 24, 2021

View reviewed changes

alexcjohnson added 17 commits July 2, 2021 11:28

hack for styled-jsx@3.4 with babel

1ea6e86

webpack minor

d69b3ca

plotly.js 2.2 changelog

0a5f054

use dash deps update branch on CI

a1dbbe4

black opt-in instead of opt-out

9a84e8f

install webpack update

f15988d

update highlight override test

f4c77e0

drop py37 tests

44aac1e

matches what we're doing in dash - 27, 36, 39

plotly.js 2.2.1 and update package-lock to npm 7

59f56c5

fix tests with assets

5178392

changelog for plotly.js 2.2.1

609024e

roll back react-jsx-parser to a known good version

5502176

and update react-markdown to latest in v4

gitignore julia entry point

3a535bd

renumber slider tests to be unique

eb7d677

a few more minor dep updates

df5ac65

fixed width for slider tooltip percy tests

7a519bf

fixup plotlyjs 2.2.1 changelog

3af946f

alexcjohnson reviewed Jul 8, 2021

View reviewed changes

centralize test assets for percy

98d617e

alexcjohnson reviewed Jul 8, 2021

View reviewed changes

alexcjohnson requested review from eff-kay and removed request for rpkyle July 8, 2021 01:18

alexcjohnson mentioned this pull request Jul 8, 2021

Deps update 20210628 plotly/dash#1680

Merged

eff-kay reviewed Jul 8, 2021

View reviewed changes

.circleci/config.yml Show resolved Hide resolved

eff-kay reviewed Jul 8, 2021

View reviewed changes

webpack.config.js Show resolved Hide resolved

eff-kay reviewed Jul 8, 2021

View reviewed changes

webpack.config.js Show resolved Hide resolved

alexcjohnson merged commit 5c20c14 into dev Jul 8, 2021

alexcjohnson deleted the update-toolchain-20210322 branch July 8, 2021 21:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update toolchain #939

Update toolchain #939

Marc-Andre-Rivet commented Mar 24, 2021 •

edited by alexcjohnson

Loading

Marc-Andre-Rivet Mar 24, 2021

Marc-Andre-Rivet Mar 24, 2021

Marc-Andre-Rivet Mar 24, 2021

Marc-Andre-Rivet Mar 24, 2021

Marc-Andre-Rivet Mar 24, 2021

alexcjohnson Jul 8, 2021

Marc-Andre-Rivet Mar 24, 2021

Marc-Andre-Rivet Mar 24, 2021

Marc-Andre-Rivet Mar 24, 2021

alexcjohnson Jul 8, 2021

alexcjohnson Jul 8, 2021

eff-kay Jul 8, 2021

alexcjohnson Jul 8, 2021

eff-kay commented Jul 8, 2021

Update toolchain #939

Update toolchain #939

Conversation

Marc-Andre-Rivet commented Mar 24, 2021 • edited by alexcjohnson Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eff-kay commented Jul 8, 2021

Marc-Andre-Rivet commented Mar 24, 2021 •

edited by alexcjohnson

Loading