Update dependencies to latest #4496

dennisroche · 2018-04-28T14:21:18Z

Description

Added yarn.lock
Updated to react 16.3
Updated to webpack 3.11
Fixed dependency warnings
Removed obsolete dependencies

Motivation and Context

Fixes #4495

How Has This Been Tested?

Running locally in Chrome and using current automation test suite.

Checklist

My PR contains...

No code changes (src/ is unmodified: changes to documentation, CI, metadata, etc.)
Dependency changes (any modification to dependencies in package.json)
Bug fixes (non-breaking change which fixes an issue)
Improvements (misc. changes to existing features)
Features (non-breaking change which adds functionality)

My changes...

are breaking changes to a public API (config options, System API, major UI change, etc).
are breaking changes to a private API (Redux, component props, utility functions, etc.).
are breaking changes to a developer API (npm script behavior changes, new dev system dependencies, etc).
are not breaking changes.

Documentation

My changes do not require a change to the project documentation.
My changes require a change to the project documentation.
If yes to above: I have updated the documentation accordingly.

Automated tests

My changes can not or do not need to be tested.
My changes can and should be tested by unit and/or integration tests.
If yes to above: I have added tests to cover my changes.
If yes to above: I have taken care to cover edge cases in my tests.
All new and existing tests passed.

…upported.

heldersepu · 2018-04-28T14:43:43Z

Bamm! checks have failed

I get the idea behind the update to latest dependencies
but migrating to yarn should be a separate discussion and PR

dennisroche · 2018-04-28T16:12:11Z

@heldersepu fair call on yarn. i'll revert that. it is just nice to a have a deterministic resolution of dependencies between builds.

heldersepu · 2018-04-29T12:26:32Z

You got a lot of failing tests...

I think they are all related to react-markdown version 3.3.0. Maybe leave that one as it was?
Also there is a new version of deep-extend that address a Vulnerability:
https://snyk.io/vuln/npm:deep-extend:20180409

dennisroche · 2018-04-30T01:40:21Z

@heldersepu I tried reverting react-markdown to @2.5.0 however tests still fail.

I think the component isn't rendering in the test as markdown looks correct in when running locally. Perhaps with the bump to enzyme-adapter-react-16 is the common point of failure.

dennisroche · 2018-04-30T01:44:47Z

Already have bumped deep-extend to 0.5.1 which contains the fix for the vulnerability (unclechu/node-deep-extend#39)

heldersepu · 2018-04-30T02:07:24Z

I do not see your push reverting react-markdown or updating deep-extend

dennisroche · 2018-04-30T02:19:53Z

I reverted the react-markdown change as test results didn't change.

heldersepu · 2018-04-30T02:27:44Z

push the revert let's see what Travis CI shows...

the latest shows:

  264 passing (761ms)
  12 pending
  25 failing

heldersepu · 2018-04-30T02:52:47Z

Yep, still see the same amount of errors...
I did test on my local only updating react-markdown and was getting a bunch of those:
Markdown Script Sanitization

heldersepu · 2018-04-30T13:50:40Z

package.json

@@ -18,7 +18,6 @@
    "build-bundle": "webpack --config webpack-dist-bundle.config.js --colors",
    "build-core": "webpack --config webpack-dist.config.js --colors",
    "build-standalone": "webpack --config webpack-dist-standalone.config.js --colors",
-    "predev": "npm install",


Why are you removing this?

Mistakenly removed. I'll restore it.

heldersepu · 2018-04-30T13:51:43Z

package.json

    "nightwatch": "^0.9.16",
    "node-sass": "^4.5.0",
-    "npm-run-all": "4.0.2",
+    "npm-run-all": "^3.1.1",


You are actually downgrading here...

That happened when I reverted from yarn. I'll fix.

heldersepu · 2018-04-30T13:52:21Z

package.json

-    "imports-loader": "0.7.1",
-    "json-loader": "0.5.4",
-    "json-server": "^0.11.0",
-    "karma": "^1.7.0",


Is karma not needed anymore?

It didn't appear to be in use; checked for usage and only found mocha testing.

webron · 2018-05-01T00:56:27Z

Can't speak for the other dependencies (leaving those to @shockey) but updating to React 16 may not be feasible right now.

dennisroche · 2018-05-01T03:23:20Z

i thought that i found the issue the missing commonmark peer dependency.

i'll rollback to react@15.6.2.

…pector with react-inspector (16.x support)

heldersepu · 2018-05-01T15:14:47Z

After latest commits we still see the same on Travis:

  264 passing (711ms)
  12 pending
  25 failing

If I was you I will close this PR and start a new one, there I will push one dependency change at a time that way you see where the UnitTests are breaking.

shockey · 2018-05-01T22:17:25Z

Hi @dennisroche! Thanks for this PR.

Added yarn.lock

Officially, our projects use npm. I have absolutely nothing against using or supporting Yarn here, but for simplicity's sake I think it's important that we stick with one or the other.

Looking at the package-lock.json vs yarn.lock discussion over at Yarn (yarnpkg/yarn#3614), the case of having both npm and yarn lockfiles present is clearly an unresolved issue: neither tool will honor the other's lockfile.

We did introduce a package-lock.json last year (#3160), but then ran into a nasty npm issue (npm/npm#16839) so we walked back the addition (0ab6fde).

All that being said: I welcome the reintroduction of lockfiles (they are clearly a good idea), but we should stick with npm instead of moving just one thing over to Yarn, specially when supporting Yarn will either be at the expense of the npm experience (npm users get no lockfile) or at the expense of consistency (npm and yarn users install from different, possibly differing, lockfiles). I'd be fine with a package-lock.json in this PR.

Feel free to propose a wholesale switch to Yarn in another PR, though 😉

Updated to react 16.3

Our React dependency has been a constant thorn in the project's side... here's some context.

Swagger-UI is not a React library: we expose a regular function that, behind the scenes, mounts a React 15 application at a specified DOM element. This means that we need to bring our own React with us, so we can run in non-React contexts (for example, https://github.com/shockey/swagger-ui-angular4/blob/master/package.json#L26).

This is great for most folks:

Non-React projects, or folks not using a front-end framework at all, don't need to know/care that we use React
React 15 users have our React dependency deduced with their own, so everything uses the same React

It breaks down with projects using React 16 and beyond (or 14 and earlier, for that matter): the top-level React in their application freaks out (something like Only a ReactOwner can have refs) when a different copy of React (ours) creates a component tree within the user application's component tree.

Changing our React version to 16 would just shift who we support: folks on 15 and earlier would suddenly have the problems described above. That would be a breaking change, which we'd like to avoid.

I've discussed this at length elsewhere in the issue tracker: see #3158, #3000 (comment), #4232 (comment), #3934 (comment), #3955 (comment).

Updated to webpack 3.11
Fixed dependency warnings
Removed obsolete dependencies

Great!

I'll add any other thoughts as a code review.

shockey · 2018-05-01T22:19:31Z

src/core/index.js

@@ -7,10 +7,6 @@ import ApisPreset from "core/presets/apis"
 import * as AllPlugins from "core/plugins/all"
 import { parseSearch } from "core/utils"

-if (process.env.NODE_ENV !== "production" && typeof window !== "undefined") {


Is there a specific reason that this was removed?

shockey · 2018-05-01T22:19:34Z

src/core/components/debug.jsx

@@ -16,7 +15,6 @@ export default class Debug extends React.Component {
      e.preventDefault()
      this.setState({jsonDumpOpen: !this.state.jsonDumpOpen})
    }
-    window.Perf = Perf


Is there a specific reason that this was removed?

dennisroche · 2018-05-02T03:31:55Z

@shockey thanks for the context on react and project history. i admit with this PR I did jump into deep end without discussing the changes.

the change to yarn was reverted early (not sure if you saw that). yarn's update interactive tooling made it easy but since found npm-check (https://github.com/dylang/npm-check).
i would like to reintroduce the package-lock.json as deterministic builds are great 👍. that nasty npm bug is resolved and in npm@^5.1 the package.json is now able to trump package-lock.json, so the experience much less of a headache
i'll drop back to react@15.6 given that is a breaking change. i didn't think it would be as 15.6 was transitioning version for 16.
react-addons-perf support was dropped in 16 (i think... anyways, that will be coming back)
i'll make sure to keep the deep-extend@^0.5.1 as contains the fix for a vulnerability

i'm going to close this PR for now and make the above changes.

thanks for the time guys.

shockey · 2018-05-02T07:11:08Z

@dennisroche no worries, we appreciate the enthusiasm 😄 there’s really no way you could’ve known all of that context ahead of time. In the future, if you’re unsure about the impact of a change that you’d like to make, feel free to drop me an email (address in profile).

Looking forward to the PRs - thanks again!

dennisroche · 2018-05-11T12:17:07Z

@shockey finally pushed the new update, see #4543

Dennis Roche added 4 commits April 28, 2018 22:10

Migrate to yarn and fix warnings

1e1aba5

Update dependencies (safe)

a03a3c0

Update to React 16.x and latest dependencies. react-addons-perf not s…

371c893

…upported.

Update webpack to v3

3913cbb

Dennis Roche added 3 commits April 29, 2018 14:00

Revert back to npm

dfa3df3

Get tests running again

e5a3253

Add package-lock.json

a1f56f4

Update deep-extend to 0.5.1 to fix vulnerability

2e27d0c

Revert react-markdown to 2.5.0

3e40ca7

heldersepu reviewed Apr 30, 2018

View reviewed changes

webron requested a review from shockey April 30, 2018 22:41

Readded predev script

17298f7

Added missing commonmark peer dependency and replace react-object-ins…

079992b

…pector with react-inspector (16.x support)

shockey reviewed May 1, 2018

View reviewed changes

dennisroche closed this May 2, 2018

dennisroche mentioned this pull request May 11, 2018

Bug/update dependencies #4543

Merged

17 tasks

dennisroche deleted the bug/update-dependencies-to-latest branch May 11, 2018 12:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update dependencies to latest #4496

Update dependencies to latest #4496

dennisroche commented Apr 28, 2018

heldersepu commented Apr 28, 2018 •

edited

Loading

dennisroche commented Apr 28, 2018

heldersepu commented Apr 29, 2018 •

edited

Loading

dennisroche commented Apr 30, 2018 •

edited

Loading

dennisroche commented Apr 30, 2018

heldersepu commented Apr 30, 2018

dennisroche commented Apr 30, 2018

heldersepu commented Apr 30, 2018 •

edited

Loading

heldersepu commented Apr 30, 2018

heldersepu Apr 30, 2018

dennisroche May 1, 2018

dennisroche May 1, 2018

heldersepu Apr 30, 2018

dennisroche May 1, 2018

dennisroche May 1, 2018

heldersepu Apr 30, 2018

dennisroche May 1, 2018 •

edited

Loading

webron commented May 1, 2018

dennisroche commented May 1, 2018 •

edited

Loading

heldersepu commented May 1, 2018

shockey commented May 1, 2018

shockey May 1, 2018

shockey May 1, 2018

dennisroche commented May 2, 2018

shockey commented May 2, 2018

dennisroche commented May 11, 2018

Update dependencies to latest #4496

Update dependencies to latest #4496

Conversation

dennisroche commented Apr 28, 2018

Description

Motivation and Context

How Has This Been Tested?

Checklist

My PR contains...

My changes...

Documentation

Automated tests

heldersepu commented Apr 28, 2018 • edited Loading

dennisroche commented Apr 28, 2018

heldersepu commented Apr 29, 2018 • edited Loading

dennisroche commented Apr 30, 2018 • edited Loading

dennisroche commented Apr 30, 2018

heldersepu commented Apr 30, 2018

dennisroche commented Apr 30, 2018

heldersepu commented Apr 30, 2018 • edited Loading

heldersepu commented Apr 30, 2018

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

dennisroche May 1, 2018 • edited Loading

Choose a reason for hiding this comment

webron commented May 1, 2018

dennisroche commented May 1, 2018 • edited Loading

heldersepu commented May 1, 2018

shockey commented May 1, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dennisroche commented May 2, 2018

shockey commented May 2, 2018

dennisroche commented May 11, 2018

heldersepu commented Apr 28, 2018 •

edited

Loading

heldersepu commented Apr 29, 2018 •

edited

Loading

dennisroche commented Apr 30, 2018 •

edited

Loading

heldersepu commented Apr 30, 2018 •

edited

Loading

dennisroche May 1, 2018 •

edited

Loading

dennisroche commented May 1, 2018 •

edited

Loading