Skip to content

fix package-lock.json to restore heroku deploys #3904

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

Merged
merged 18 commits into from
Aug 24, 2019
Merged

Conversation

calebcartwright
Copy link
Member

@calebcartwright calebcartwright commented Aug 22, 2019

Heroku deploys are failing again with missing dependencies that seem to be related to the package-lock.json file

#3898 (comment)
https://dashboard.heroku.com/pipelines/d8fd42e8-8e42-466e-98fc-f5f50edbdd8f

@shields-cd shields-cd had a problem deploying to shields-staging-pr-3904 August 22, 2019 23:00 Failure
@shields-ci
Copy link

shields-ci commented Aug 22, 2019

Warnings
⚠️

This PR modified package.json, but not package-lock.json - Perhaps you need to run npm install?

Messages
📖 ✨ Thanks for your contribution to Shields, @calebcartwright!

Generated by 🚫 dangerJS against 937fa4a

@calebcartwright
Copy link
Member Author

No such luck 🤔 I'm gonna see if I can get dependabot to open a PR with a fix

@shields-cd shields-cd had a problem deploying to shields-staging-pr-3904 August 22, 2019 23:41 Failure
@shields-cd shields-cd had a problem deploying to shields-staging-pr-3904 August 22, 2019 23:55 Failure
@paulmelnikow
Copy link
Member

@calebcartwright Have you tried updating to the latest npm? I’m not following development super closely but there were some recent regressions on Windows that were fixed in the latest version.

@paulmelnikow
Copy link
Member

I'll give this a second shot. I think the last known working deploy was 316749b so I'll start by reverting to that.

@paulmelnikow
Copy link
Member

I wonder if this is not a problem with the lockfile, but with the npm installer. If I blow away my local node_modules and re-run npm install, I can reproduce the problem locally.

@paulmelnikow
Copy link
Member

From the build log:

       We're sorry this build is failing! You can troubleshoot common issues here:
       https://devcenter.heroku.com/articles/troubleshooting-node-deploys
       
       Some possible problems:
       
       - Dangerous semver range (>) in engines.node
         https://devcenter.heroku.com/articles/nodejs-support#specifying-a-node-js-version

@shields-cd shields-cd had a problem deploying to shields-staging-pr-3904 August 23, 2019 13:53 Failure
@calebcartwright
Copy link
Member Author

If I blow away my local node_modules and re-run npm install, I can reproduce the problem locally.

Ditto. I just remembered that last time I ran into this I had to delete the node_modules directory in the root of the project as well as the one within the gh-badges directory.

🤞

@calebcartwright
Copy link
Member Author

I'm on node 8.x and npm 5.x at the moment. I have node 10 on Windows so I guess I can give that a try too, especially since nothing else seems to be working 😄

@shields-cd shields-cd had a problem deploying to shields-staging-pr-3904 August 23, 2019 14:10 Failure
@paulmelnikow
Copy link
Member

I wonder if I was wrong about which was the last known working version.

@calebcartwright
Copy link
Member Author

I'm baffled. I know this is the most unhelpful thing commonly said by engineers, but it works on my machine!

@paulmelnikow
Copy link
Member

Can you give me steps to produce a good install on this commit? I can try it on my machine.

The commit on #3909 is not working on my machine.

@calebcartwright
Copy link
Member Author

For me a rm -rf on node_modules and gh-badges/node-modules and then an npm i and npm start. I can also run an npm ci in there

@shields-cd shields-cd had a problem deploying to shields-staging-pr-3904 August 24, 2019 14:58 Failure
@shields-cd shields-cd had a problem deploying to shields-staging-pr-3904 August 24, 2019 15:24 Failure
@shields-cd shields-cd had a problem deploying to shields-staging-pr-3904 August 24, 2019 15:40 Failure
@shields-cd shields-cd temporarily deployed to shields-staging-pr-3904 August 24, 2019 18:31 Inactive
@calebcartwright
Copy link
Member Author

Cool! Yea, by default env vars don’t propagate from the staging app to the review apps (which is a good thing, really).

Definitely! I had explicitly specified my review app though (heroku config:set CYPRESS_INSTALL_BINARY=0 --app shields-staging-pr-3904) which I figured would have worked.

I also tried setting them in app.json but they don't seem to be working their either 🤷‍♂

@shields-cd shields-cd temporarily deployed to shields-staging-pr-3904 August 24, 2019 19:13 Inactive
@calebcartwright
Copy link
Member Author

Even using strings for the env values in app.json didn't seem to have an impact

...
[19:16:11]  Unzipping Cypress        98% 1s [title changed]
       [19:16:11]  Unzipping Cypress        99% 1s [title changed]
       [19:16:12]  Unzipping Cypress        100% 0s [title changed]
       [19:16:12]  Unzipped Cypress        [title changed]
       [19:16:12]  Unzipped Cypress        [completed]
       [19:16:12]  Finishing Installation  [started]
       [19:16:12]  Finished Installation   /app/.cache/Cypress/3.4.1 [title changed]
       [19:16:12]  Finished Installation   /app/.cache/Cypress/3.4.1 [completed]
       �[?25h
       You can now open Cypress by running: node_modules/.bin/cypress open
       
       https://on.cypress.io/installing-cypress

@paulmelnikow
Copy link
Member

Gosh, so much strangeness!

I'm guessing the problem we are running into is a recent regression in npm, and something we ought to report so it can be fixed. I haven't had much time at the keyboard in recent weeks and haven't had too much time to dig into this myself. Though its effects are kind of far-reaching (e.g. #3901) and I feel like we need to get to the bottom of it.

@calebcartwright
Copy link
Member Author

Perhaps this is related? https://npm.community/t/6-11-1-some-dependencies-are-no-longer-being-installed/9586

@shields-cd shields-cd had a problem deploying to shields-staging-pr-3904 August 24, 2019 19:45 Failure
@paulmelnikow paulmelnikow had a problem deploying to shields-staging-pr-3904 August 24, 2019 19:53 Failure
@shields-cd shields-cd temporarily deployed to shields-staging-pr-3904 August 24, 2019 20:06 Inactive
@paulmelnikow paulmelnikow temporarily deployed to shields-staging-pr-3904 August 24, 2019 20:14 Inactive
@paulmelnikow
Copy link
Member

It looks like that is the solution: npm install -g [email protected] makes the problem go away.

Steps to reproduce the problem:

npm install -g [email protected]
rm -rf node_modules
npm install
ls -d node_modules/is-css-color

@calebcartwright
Copy link
Member Author

I can't approve this as the author but 👍 from me

@calebcartwright calebcartwright merged commit 449e3d1 into master Aug 24, 2019
@shields-deployment
Copy link

This pull request was merged to master branch. This change is now waiting for deployment, which will usually happen within a few days. Stay tuned by joining our #ops channel on Discord!

After deployment, changes are copied to gh-pages branch:

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.

4 participants