Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update NPM Packages #583

Closed
wants to merge 8 commits into from
Closed

Update NPM Packages #583

wants to merge 8 commits into from

Conversation

DuncanBetts
Copy link
Contributor

@DuncanBetts DuncanBetts commented Sep 24, 2019

I've broken down my changes into a series of commits, to re-state what each does:

  1. Perform npm update
  2. Perform npm audit fix
  3. Installation of peer dependency, in response to error.
  4. Removal of unused packages (as suggested by nvx npm-check) - I attempted to double-check these were indeed unused, too by searching the codebase for mention of the packages (and didn't follow every recommendation).
  5. package updates suggested by "npm-check -u", claimed to be non-breaking only
  6. "npm start" was failing at this point, so as suggested here updated web3.
  7. Realized "npm run test" was failing so added back a required package previously removed in step 4.

I've subsequently clicked around the site and things appear to be working (at least in Chrome, on my machine...).

However, am quite happy to have this rejected - maybe my methodology (or some part of my methodology) around updating packages is wrong? Specifically:

Although both npm nor npm-check claim that the changes they'll have made will be non-breaking only, I don't know to what extent that can be relied upon.

Duncan Betts added 8 commits September 24, 2019 10:18
(130) _ ~/.../js/nexchange-open-client-react
% parcel build src/index.js                                                                                                                                                                                                                                                              (feature/implement-parcel)
🚨  /home/_/dev/js/nexchange-open-client-react/src/index.js: Cannot resolve dependency '/home/_/dev/js/nexchange-open-client-react/node_modules/@babel/runtime/helpers/esm/extends' at '/home/_/dev/js/nexchange-open-client-react/node_modules/@babel/runtime/helpers/esm/extends'
    at Resolver.resolve (/home/_/.npm/lib/node_modules/parcel-bundler/src/Resolver.js:71:17)
Double checked this, before uninstalling them with the following
command:

npm uninstall start-server-and-test react-test-renderer jest-mock-axios jest-localstorage-mock @storybook/addons @storybook/addon-links webpack-dev-middleware reactstrap react-svg-inline react-loadable js-yaml expose-loader @babel/runtime-corejs2
As suggested here oasislabs/web3c.js#93
@DuncanBetts
Copy link
Contributor Author

DuncanBetts commented Sep 24, 2019

Until the final commit 26bc4b5 with updated snapshots, tests are failing.

I manually diffed these snapshots and they look OK to me but haven't double checked what these changes enzyme have made are. I manually tested too, but not the entire app.

@DuncanBetts
Copy link
Contributor Author

DuncanBetts commented Sep 24, 2019

Have looked at Travis CI build failures but they look like the errors that "normally occur".

@BeOleg BeOleg requested a review from MMrj9 October 13, 2019 21:39
@elis
Copy link
Contributor

elis commented Nov 13, 2019

Deferred by @BeOleg

@elis elis closed this Nov 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants