Upgraded NPM dependencies

noisecapella commented 8 years ago

What are the relevant tickets?

Related to #476

What's this PR do?

Upgrades dependencies and removes unused ones. In order to fix compatibility issues certain changes were made:

shopping cart is now opened using a prop instead of our hacky workaround
ReviewsTab is removed, which wasn't used anyway
onEnterKeyDown was removed in LoginModal and replaced with onKeyDown according to the warning's recommendation
background-color is marked !important to fix the blue background issue which is the original reason we haven't upgraded material-ui
tests are updated to click the material-ui button properly
Where should the reviewer start?

package.json

How should this be manually tested?

The reviewer should go through the checkout workflow and make sure everything looks right. In addition they should also use the enter key on every field in the login modal to confirm that it still works as expected. (This is not covered by tests unfortunately.)

Any background context you want to provide?

This also pins dependencies to exact versions to avoid issues in the future.

Screenshots (if appropriate)

What GIF best describes this PR or how it makes you feel?

alicewriteswrongs commented 8 years ago

@noisecapella I'm curious why we're pinning exact dependency versions instead of using a shinkwrap.json, have there been problems with it before? (I wouldn't be surprised, npm is just about the worst behaved piece of tooling ever :P )

noisecapella commented 8 years ago

We had a shrinkwrap file but we didn't do the work of keeping it up to date with the versions in package.json. What advantage does shrinkwrap provide over exact versions?

justinabrahms commented 8 years ago

https://github.com/mitodl/teachersportal/pull/477 -- context

alicewriteswrongs commented 8 years ago

yeah, I don't really think shrinkwrap is a better solution - it's supposed to work something like Gemfile.lock for bundler, but it's basically just worse, haha. It also produces enormous diffs - when you bump package versions on a decently sized project you'll often get diffs in the 2-3k line range.

was just wondering what the previous experience was, since I haven't done the exactly version pinning in package.json solution before.

bdero commented 8 years ago

FYI I'm trying to get the review app working for this on Heroku right now.

bdero commented 8 years ago

The review apps work again: https://teachersportal-ci-pr-478.herokuapp.com/

noisecapella commented 8 years ago

@aliceriot Are you running npm install from within the docker container?

alicewriteswrongs commented 8 years ago

@noisecapella yep! I'm rebuilding my docker images right now, I'll see if that changes it.

noisecapella commented 8 years ago

Cool, I haven't seen that problem on my container. I just force pushed but there are no additional changes FYI, just a rebase on master

alicewriteswrongs commented 8 years ago

@noisecapella still happening :(

I just did docker-compose build and then docker-compose up.

in the watch container:

lodash_why

in the browser (when I visit localhost:8075):

lodash_browser

doesn't look like things are rendering correctly, I just get a blank white page with the 'Help' icon at the bottom.

is there a setup step I might have missed?

alicewriteswrongs commented 8 years ago

fixed!

noisecapella commented 8 years ago

Any other comments @aliceriot?

alicewriteswrongs commented 8 years ago

nope, it's a :+1: from me!

noisecapella commented 8 years ago

Thanks @aliceriot!

mitodl / teachersportal