Update requirements to support React 16 #165

mjclawar · 2018-02-24T14:58:57Z

This PR addresses all of the warnings raised by React v15.6 in dash-core-components.

Changed

Update versions for:
- react-dates to "16.3.2"
- react-dropzone to "4.2.8"
- react-markdown to "3.2.1"
- react-select to "1.2.1"
- react-select-fast-filter-options to "0.2.3"
- react-syntax-highlighter to "7.0.0"
- react-virtualized-select to "3.1.3"
- react and react-dom as peerDependencies "^15.4.0 || ^16.0.0"
Add style-loader and css-loader to webpack babel loaders to support
new version of react-dates with separate css file
Change import location from react-syntax-highlighter to:

import {arduinoLight, monokai} from 'react-syntax-highlighter/styles/hljs';

Change import location from react-virtualized-select to:

import ReactDropdown from 'react-virtualized-select/dist/umd/react-virtualized-select';

Closes #164

Passes all existing tests with React v15.4.
Also works locally with dash-html-components using React v16 🎉 (note that to test React v16 locally at this moment requires the dash-renderer patch proposed in plotly/dash-renderer#45 that adds optional React 16 support and moves PropTypes to prop-types)

Additional discussion

@chriddyp I did end up having to change some of the imports as noted above to support the version updates. Existing tests still pass, but I'm not certain that you would approve of all of the structural changes, especially adding additional loaders/an alias to webpack here

…yles location for react-syntax-highlighter. Comment out Dropdown to ensure that all other components load correctly with React 15.6

… dependencies handled by dash-components-archetype package.json

… react-virtualized-select

chriddyp · 2018-02-26T18:41:27Z

Passes all existing tests with React v15.4.

Very nice! So this is not a breaking change, correct?

chriddyp · 2018-02-26T18:46:40Z

Once plotly/dash-renderer#45 is merged, perhaps we can run our tests twice, once for each version of react.

chriddyp · 2018-02-26T18:47:02Z

@mjclawar - This looks good to me. Is there anything else that you would like to add?

mjclawar · 2018-02-26T19:26:10Z

Very nice! So this is not a breaking change, correct?

It is not a breaking change from the dash developer's perspective, except I can't absolutely guarantee that the underlying stylesheets for, e.g., react-syntax-highlighter have not changed. I looked at the tests that ran and everything seemed to be styled correctly, however.

...perhaps we can run our tests twice, once for each version of react.

That's a really good idea. Want me to add that in to this PR when you merge in plotly/dash-renderer#45?

# Conflicts: # CHANGELOG.md # dash_core_components/version.py # package.json

chriddyp · 2018-03-29T02:29:12Z

@mjclawar - Now that plotly/dash-renderer#45 is merged and released, would you mind making an integration test to test both versions? Then, I'll be happy to merge and release 😸

# Conflicts: # CHANGELOG.md # dash_core_components/version.py # package.json

Update requirements-dev to require at least dash-renderer v0.12.1

…move dash-core-components dependency

mjclawar · 2018-03-29T19:13:21Z

@chriddyp Updated integration tests to test:

upload gallery
gallery
location link
NOT dash-table-experiments, since those tests needs to be updated for the new React (the same prop-types codemod, etc.)

The way I did this was modify the self.startServer method to take an optional react_version argument, which calls the dash_renderer._set_react_version method before it starts up the server.

Then those integration tests have a test that handles the exact same behavior as before with the older React version, such as in test_location_link (with the same snapshot names). I added another test, like:

def test_location_link_v16(self):
    self.create_test_location_link(react_version='16.2.0')

This test updates the react and react-dom versions served to v16.2.0, which creates an additional with a name like "my-previous-snapshot-namev16.2.0", via something like:

self.snapshot(self.snapshot_name('link -- /test/pathname/a?queryA=valueA', react_version))

chriddyp

This looks good! A couple of questions about how this will affect user-supplied CSS.
It looks like the image tests on percy aren't running because of some circleci settings (env variables aren't getting set on 3rd party pull requests) - I've just updated that setting and I'll try making another build.
I'm also going to make a prerelease version and test it on the dash-docs repo.

chriddyp · 2018-03-30T16:17:04Z

CHANGELOG.md

+ - `react-dates` to `"16.3.2"`
+ - `react-dropzone` to `"4.2.8"`
+ - `react-markdown` to `"3.2.1"`
+ - `react-select` to `"1.2.1"`


Since we're bumping major versions in these libraries, I bet that this will introduce some breaking changes in the component's CSS. In case any users are overriding this CSS, then their overrides won't work anymore. So, I think we should make a breaking change version bump to 1.0.0 (according to semver)

And let's leave a note in the changelog that mentions that CSS stylesheets may be different

I haven't actually looked into whether or not the CSS or markup has changed in these versions, I'm just guessing that it has.

OK, will update to 1.0.0.

chriddyp · 2018-03-30T16:18:44Z

requirements-dev.txt

@@ -1,5 +1,5 @@
 dash_html_components
-dash_renderer
+dash_renderer>=0.12.1


Is this just for the React 16 change?

Yes, otherwise can't use normal API to test React 16 version tests. Can shim something in for the tests to not update the version if you prefer.

chriddyp · 2018-03-30T16:20:26Z

src/components/DatePickerRange.react.js

@@ -1,3 +1,5 @@
+import 'react-dates/initialize'; // https://github.com/airbnb/react-dates/issues/750#issuecomment-335013909
+import 'react-dates/lib/css/_datepicker.css'; // react-dates css


The CSS loaders will inject the CSS into the head with JS, is that right? I wonder if this will make it harder to override these stylesheets (i.e. will user-added stylesheets get added before or after the injected stylesheets?)

The CSS loaders will inject the CSS into the head with JS, is that right?

Yes, it makes <style> tags in the head, which will show up after the stylesheets.
Should I instead remove this, plus the css-loader plugin for Webpack, and insert the newer react-dates CSS stylesheet? I hadn't realized that this was happening for serving CSS, as you note below:

Er, looks like we would only need to remove react-dates. However, we will need to update the stylesheets for the rest of the components that had their versions bumped. We'll also need to include those new stylesheets in the dash_core_components/ folder.

Yeah, let's keep the traditional stylesheets for now, that way it's easier for users to override them without removing them.

chriddyp · 2018-03-30T16:27:56Z

Oh also, can we remove the CSS files from __init__.py, the dash_core_components/ folder, and MANIFEST.in?

chriddyp · 2018-03-30T16:29:38Z

Also, I'm going to make you a contributor @mjclawar - That'll make pulling down your branches a bit easier for me :)

chriddyp · 2018-03-30T16:37:37Z

Oh also, can we remove the CSS files from init.py, the dash_core_components/ folder, and MANIFEST.in?

Er, looks like we would only need to remove react-dates. However, we will need to update the stylesheets for the rest of the components that had their versions bumped. We'll also need to include those new stylesheets in the dash_core_components/ folder.

Let me know when that's been done, and then I'll do the prerelease and test on dash-docs 😸

chriddyp · 2018-03-30T16:52:01Z

======================================================================
ERROR: test_location_link (test.test_integration.Tests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test/test_integration.py", line 490, in test_location_link
    self.create_test_location_link()
  File "test/test_integration.py", line 470, in create_test_location_link
    self.snapshot(self.snapshot_name('link -- /test/pathname/a', react_version))
  File "test/test_integration.py", line 72, in snapshot
    self.percy_runner.snapshot(name=name)
  File "/home/ubuntu/dash-core-components/.tox/py27/lib/python2.7/site-packages/percy/runner.py", line 72, in snapshot
    snapshot_data = self.client.create_snapshot(build_id, [root_resource], **kwargs)
  File "/home/ubuntu/dash-core-components/.tox/py27/lib/python2.7/site-packages/percy/client.py", line 104, in create_snapshot
    return self._connection.post(path=path, data=data)
  File "/home/ubuntu/dash-core-components/.tox/py27/lib/python2.7/site-packages/percy/connection.py", line 39, in post
    raise e
HTTPError: 400 Client Error: Bad Request for url: https://percy.io/api/v1/builds/618707/snapshots/

======================================================================
ERROR: test_location_link_v16 (test.test_integration.Tests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test/test_integration.py", line 493, in test_location_link_v16
    self.create_test_location_link(react_version='16.2.0')
  File "test/test_integration.py", line 469, in create_test_location_link
    self.snapshot('link -- /test/pathname/a')
  File "test/test_integration.py", line 72, in snapshot
    self.percy_runner.snapshot(name=name)
  File "/home/ubuntu/dash-core-components/.tox/py27/lib/python2.7/site-packages/percy/runner.py", line 72, in snapshot
    snapshot_data = self.client.create_snapshot(build_id, [root_resource], **kwargs)
  File "/home/ubuntu/dash-core-components/.tox/py27/lib/python2.7/site-packages/percy/client.py", line 104, in create_snapshot
    return self._connection.post(path=path, data=data)
  File "/home/ubuntu/dash-core-components/.tox/py27/lib/python2.7/site-packages/percy/connection.py", line 39, in post
    raise e
HTTPError: 400 Client Error: Bad Request for url: https://percy.io/api/v1/builds/618707/snapshots/

looks like there are some issues with the snapshot name for the url test

mjclawar · 2018-03-30T17:18:40Z

looks like there are some issues with the snapshot name for the url test

😡 I'll look into the formatting. Is there any way for me (or anyone) to see if the percy tests will fail/pass locally?

chriddyp · 2018-03-30T17:31:34Z

to see if the percy tests will fail/pass locally?

Yeah, I can email you the percy API keys and you can run it locally. I'll also add you to the percy organization

- Update css files in dash_core_components/ to newer versions to match package.json - Update MANIFEST.in to match newer included css - Update babel config to remove style loader and css loader

Fix integration test to avoid snapshotting twice in same test

- Update CHANGELOG to reflect 1.0.0

mjclawar · 2018-04-01T13:21:37Z

@chriddyp should be all set now:

Reverted to remove css-loader for react-dates and removed those import statements in favor of react-dates_datepicker@16.3.2.min.css (Note that this was pulled from the actual node_modules installation under lib)
Fixed integration tests which were just taking the same snapshot twice (I had missed the older snapshot format I was using since I couldn't duplicate the Percy fails locally)
Updated MANIFEST.in to include new files
Updated CHANGELOG for v1.0.0
Updated version.py for v1.0.0

I also moved CONTRIBUTING.md to a .github directory (works the same as the root directory), and added an ISSUE_TEMPLATE.md and a PULL_REQUEST_TEMPLATE.md` that will provide checklists that can prevent the versioning mismatches/lack of updates of files that I run into frequently for dash repos (because you have to update Python and JS files and I always forget one). For reference, see the GitHub docs on templates. If you do not like the defaults or any of the language, I can remove or edit either/both of the templates. In particular, I wanted to at least have these checkboxes when I open a PR:

<!--- Did you run the test suite locally? Did you add any tests for new behavior? -->
- [ ] `dash_core_components/version.py` has been updated if necessary
- [ ] `package.json` has been versioned if necessary
- [ ] `CHANGELOG.md` has a detailed new entry if necessary
- [ ] `MANIFEST.in` has been updated if non-Python files were updated in dash_core_components
- [ ] [Tests pass](https://github.com/plotly/dash-core-components/blob/master/.github/CONTRIBUTING.md#running-the-tests) on my local machine

The ISSUE_TEMPLATE.md also has a message at the top to remind everyone that the community forum is the best place for most help questions:

<!--- Thanks for starting to open an issue for dash-core-components! We appreciate you being a part of the community. -->
<!--- Note that for general help questions, https://community.plot.ly/c/dash is probably your best bet! --->
<!--- You can help make the development process better by taking some time to write a complete description of the bug or feature request. -->
<!--- Below, provide a general summary of your changes in the Title above -->

chriddyp · 2018-04-03T16:28:23Z

dash_core_components/rc-slider@8.6.0.css

@@ -4,6 +4,8 @@
 padding: 5px 0;
 width: 100%;
 border-radius: 6px;
+ -ms-touch-action: none;
+ touch-action: none;


Ah, very nice - we have the diffs in here

chriddyp · 2018-04-03T16:32:06Z

These changes look good! Thanks for updating all of the contributing stuff as well.

I'm going to make a prerelease version and test it on a couple of my projects and on the dash-docs repo. If all goes well, we should be able to release soon 🍻

chriddyp · 2018-04-03T16:48:55Z

After doing npm i, I get a few warnings:

  └── UNMET PEER DEPENDENCY webpack@^2.0.0 || ^3.0.0 || ^4.0.0

npm WARN react-codemirror@1.0.0 requires a peer of react@>=15.5 <16 but none was installed.
npm WARN react-codemirror@1.0.0 requires a peer of react-dom@>=15.5 <16 but none was installed.
npm WARN react-dates@16.4.0 requires a peer of react@^0.14 || ^15.5.4 || ^16.1.1 but none was installed.
npm WARN react-dates@16.4.0 requires a peer of react-dom@^0.14 || ^15.5.4 || ^16.1.1 but none was installed.
npm WARN schema-utils@0.4.5 requires a peer of webpack@^2.0.0 || ^3.0.0 || ^4.0.0 but none was installed.

Do you think we need to update these packages?

chriddyp · 2018-04-03T16:49:39Z

Do you think we need to update these packages?

Probably not react or react-dom since they are external, but webpack?

mjclawar · 2018-04-03T16:50:39Z

Probably not react or react-dom since they are external, but webpack?

Let me quickly check if the webpack config would need to be changed to update to v2.

# Conflicts: # CHANGELOG.md # dash_core_components/version.py # package.json

chriddyp · 2018-04-03T16:57:33Z

pip install dash-core-components==1.0.0-rc1

mjclawar · 2018-04-03T17:01:53Z

webpack config would need to be changed to update to v2.

Lots of changes (taking probably a couple of hours). So, up to you:

Make a new issue specific to bumping webpack version to at least v2 (preferably would just jump to v4 to keep up since they made big changes to the API again) and I can update all of the webpack config objects to address that issue.
Do all of these changes in this update since I suppose an argument could reasonably be made that updating the entire build config is a major update.

I don't think that the deprecation warning actually breaks anything at this point.

bpostlethwaite · 2018-04-13T02:27:46Z

I'll take a look at what is needed to bump the webpack version up to 4 and open a separate PR for that. Up to @chriddyp re: waiting to merge this PR until that is complete (or not).

mjclawar · 2018-05-26T08:53:08Z

@chriddyp We have been using dash-core-components==1.0.0-rc1 and React 16.2.0 in five separate applications with no noticeable impact for about a month now. Anything else you want for an official release?

Marc-Andre-Rivet · 2018-12-13T16:38:58Z

@mjclawar Thanks for the excellent work done here and your involvement. We now have a Dash 1.0 transition plan that includes upgrading to React 16. As such this PR will probably never be merged as-is. I will link it to the transition epic (plotly/dash#469) for future reference.

mjclawar added 5 commits February 24, 2018 07:34

Update babel to use style and css loaders for react-dates. Use new st…

e99bff2

…yles location for react-syntax-highlighter. Comment out Dropdown to ensure that all other components load correctly with React 15.6

Add back Dropdown and update package.json to remove unnecessary babel…

69415ba

… dependencies handled by dash-components-archetype package.json

Remove additional react-virtualized requirement in favor of old, only…

f2f65d4

… react-virtualized-select

v0.20.0

e02208c

0.20.0

61207be

Merge remote-tracking branch 'upstream/master' into 164-react-16

4a93f26

# Conflicts: # CHANGELOG.md # dash_core_components/version.py # package.json

Merge remote-tracking branch 'upstream/master' into 164-react-16

697a9a5

# Conflicts: # CHANGELOG.md # dash_core_components/version.py # package.json

mjclawar mentioned this pull request Mar 29, 2018

MANIFEST.in does not include React 16 bundles in v0.12.0 plotly/dash-renderer#49

Closed

mjclawar added 4 commits March 29, 2018 14:35

Update tests for non dash-table-experiments to use React v16.2.

55adf46

Update requirements-dev to require at least dash-renderer v0.12.1

Fix typing issue from colon to =

c365c6a

Update requirements-locked for dash-renderer

4bca18b

Update requirements-locked to newer dash and dash-html-components, re…

6a7a38b

…move dash-core-components dependency

chriddyp reviewed Mar 30, 2018

View reviewed changes

mjclawar added 5 commits April 1, 2018 08:43

- Remove css-loader package.json requirement

09ead14

- Update css files in dash_core_components/ to newer versions to match package.json - Update MANIFEST.in to match newer included css - Update babel config to remove style loader and css loader

Update versions in dash_core_components.__init__ for css

47eb34c

Fix integration test to avoid snapshotting twice in same test

- Update to v1.0.0 for breaking CSS changes

e9976e8

- Update CHANGELOG to reflect 1.0.0

1.0.0

be35156

Merge remote-tracking branch 'upstream/master' into 164-react-16

738c854

Update .github for PR and issue templates

4bcf2f8

chriddyp reviewed Apr 3, 2018

View reviewed changes

Merge remote-tracking branch 'upstream/master' into 164-react-16

fb9c27b

# Conflicts: # CHANGELOG.md # dash_core_components/version.py # package.json

Update from master

8001b95

Marc-Andre-Rivet closed this Dec 13, 2018

chriddyp mentioned this pull request Dec 13, 2018

Dash 1.0.0 Series - Proposed Set of *Breaking Changes* plotly/dash#469

Closed

24 tasks

mjclawar deleted the 164-react-16 branch December 18, 2018 17:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update requirements to support React 16 #165

Update requirements to support React 16 #165

mjclawar commented Feb 24, 2018

chriddyp commented Feb 26, 2018 •

edited

Loading

chriddyp commented Feb 26, 2018

chriddyp commented Feb 26, 2018

mjclawar commented Feb 26, 2018

chriddyp commented Mar 29, 2018

mjclawar commented Mar 29, 2018

chriddyp left a comment

chriddyp Mar 30, 2018

chriddyp Mar 30, 2018

chriddyp Mar 30, 2018

mjclawar Mar 30, 2018

chriddyp Mar 30, 2018

mjclawar Mar 30, 2018

chriddyp Mar 30, 2018

mjclawar Mar 30, 2018

chriddyp Mar 30, 2018 •

edited

Loading

mjclawar Mar 30, 2018

chriddyp commented Mar 30, 2018

chriddyp commented Mar 30, 2018

chriddyp commented Mar 30, 2018

chriddyp commented Mar 30, 2018

mjclawar commented Mar 30, 2018

chriddyp commented Mar 30, 2018

mjclawar commented Apr 1, 2018 •

edited

Loading

chriddyp Apr 3, 2018

chriddyp commented Apr 3, 2018

chriddyp commented Apr 3, 2018

chriddyp commented Apr 3, 2018

mjclawar commented Apr 3, 2018 •

edited

Loading

chriddyp commented Apr 3, 2018

mjclawar commented Apr 3, 2018

bpostlethwaite commented Apr 13, 2018

mjclawar commented May 26, 2018

Marc-Andre-Rivet commented Dec 13, 2018

		@@ -1,3 +1,5 @@
		import 'react-dates/initialize'; // https://github.com/airbnb/react-dates/issues/750#issuecomment-335013909
		import 'react-dates/lib/css/_datepicker.css'; // react-dates css

Update requirements to support React 16 #165

Update requirements to support React 16 #165

Conversation

mjclawar commented Feb 24, 2018

Changed

Additional discussion

chriddyp commented Feb 26, 2018 • edited Loading

chriddyp commented Feb 26, 2018

chriddyp commented Feb 26, 2018

mjclawar commented Feb 26, 2018

chriddyp commented Mar 29, 2018

mjclawar commented Mar 29, 2018

chriddyp left a 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

chriddyp Mar 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chriddyp commented Mar 30, 2018

chriddyp commented Mar 30, 2018

chriddyp commented Mar 30, 2018

chriddyp commented Mar 30, 2018

mjclawar commented Mar 30, 2018

chriddyp commented Mar 30, 2018

mjclawar commented Apr 1, 2018 • edited Loading

Choose a reason for hiding this comment

chriddyp commented Apr 3, 2018

chriddyp commented Apr 3, 2018

chriddyp commented Apr 3, 2018

mjclawar commented Apr 3, 2018 • edited Loading

chriddyp commented Apr 3, 2018

mjclawar commented Apr 3, 2018

bpostlethwaite commented Apr 13, 2018

mjclawar commented May 26, 2018

Marc-Andre-Rivet commented Dec 13, 2018

chriddyp commented Feb 26, 2018 •

edited

Loading

chriddyp Mar 30, 2018 •

edited

Loading

mjclawar commented Apr 1, 2018 •

edited

Loading

mjclawar commented Apr 3, 2018 •

edited

Loading