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

Node.js 14 / NPM 6 - Using dep with npm: prefix can result in invalid lockfiles #9654

Closed
1 of 4 tasks
patrickleet opened this issue Apr 20, 2021 · 18 comments
Closed
1 of 4 tasks
Labels
auto:reproduction A minimal reproduction is necessary to proceed manager:npm package.json files (npm/yarn/pnpm) status:requirements Full requirements are not yet known, so implementation should not be started type:bug Bug fix of existing functionality

Comments

@patrickleet
Copy link

patrickleet commented Apr 20, 2021

What Renovate type, platform and version are you using?

Github

Describe the bug

The generated package-lock file from renovate is sometimes invalid.

I've been using renovate for years, only seeing this for the first time in the past couple days.

When running npm ci in the CI process, and when cloning it down locally to verify, I get this error:

npm ci        
npm WARN prepare removing existing node_modules/ before installation
npm ERR! Cannot read property 'fetchSpec' of undefined

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/patrickleet/.npm/_logs/2021-04-20T15_08_26_091Z-debug.log

The logs from that log file are:

cat /Users/patrickleet/.npm/_logs/2021-04-20T15_08_26_091Z-debug.log0 info it worked if it ends with ok
1 verbose cli [ '/usr/local/bin/node', '/usr/local/bin/npm', 'ci' ]
2 info using npm@6.14.13
3 info using node@v14.16.1
4 verbose npm-session 49f46ed1129803aa
5 info prepare initializing installer
6 verbose prepare starting workers
7 verbose prepare installation prefix: /Users/patrickleet/dev/rivi/engineering-meta/apps/projects/connect
8 verbose prepare using package-lock.json
9 warn prepare removing existing node_modules/ before installation
10 verbose checkLock verifying package-lock data
11 verbose teardown shutting down workers.
12 info teardown Done in 0s
13 verbose stack TypeError: Cannot read property 'fetchSpec' of undefined
13 verbose stack     at /usr/local/lib/node_modules/npm/node_modules/lock-verify/index.js:41:52
13 verbose stack     at Array.forEach (<anonymous>)
13 verbose stack     at /usr/local/lib/node_modules/npm/node_modules/lock-verify/index.js:25:25
14 verbose cwd /Users/patrickleet/dev/rivi/engineering-meta/apps/projects/connect
15 verbose Darwin 20.3.0
16 verbose argv "/usr/local/bin/node" "/usr/local/bin/npm" "ci"
17 verbose node v14.16.1
18 verbose npm  v6.14.13
19 error Cannot read property 'fetchSpec' of undefined
20 verbose exit [ 1, true ]

Running npm i again patches the lock file and it then works.

Have you created a minimal reproduction repository?

No, I'm seeing it on multiple repositories, but they are private.

In the example I'm seeing now it's babel-monorepo being updated, but I don't think that's anything to do with the issue as I saw it on another dep as well.

If it's needed I can try to create something, but I don't even know that it'll happen. I use renovate on a lot of repos in many orgs to do the same thing.

Did the way lockfiles get generated change recently? Maybe something to do with npm6/7 which use different lockfile versions? It doesn't change the lockfile version so that seems unlikely..

Please read the minimal reproductions documentation to learn how to make a good minimal reproduction repository.

I do have an .npmrc for this project but I don't think that's the issue either?

  • I have provided a minimal reproduction repository
  • I don't have time for that, but it happens in a public repository I have linked to
  • I don't have time for that, and cannot share my private repository
  • The nature of this bug means it's impossible to reproduce publicly

Additional context

@patrickleet patrickleet added priority-5-triage status:requirements Full requirements are not yet known, so implementation should not be started type:bug Bug fix of existing functionality labels Apr 20, 2021
@patrickleet
Copy link
Author

patrickleet commented Apr 20, 2021

oddly this is literally the only diff in the package-lock.json file after cloning down the branch and running npm i

image

And that makes npm ci work again.

So maybe it's something to do with this dependency format:

"react-dom": "npm:@hot-loader/react-dom@16.14.0",

I'll see about changing that?

@rarkins rarkins added auto:reproduction A minimal reproduction is necessary to proceed manager:npm package.json files (npm/yarn/pnpm) labels Apr 20, 2021
@github-actions
Copy link
Contributor

Hi there,

The Renovate team needs your help! Before we can start work on your issue we first need to know exactly what's causing the current behavior. A minimal reproduction helps us with this.

To get started, please read our guide on minimal reproductions to understand what is needed.

We may close the issue if you have not provided a minimal reproduction within two weeks. If you need more time, or are stuck, please ask for help or more time in a comment.

Good luck,

The Renovate team

@rarkins
Copy link
Collaborator

rarkins commented Apr 20, 2021

Unfortunately there's nothing about your description or the npm ci logs which would get us any close to reproducing or fixing. If you have the debug logs from when such a "bad" commit is made then maybe we can guess at it, but otherwise not.

@patrickleet
Copy link
Author

patrickleet commented Apr 20, 2021

There was a chance you've seen it before.

I think it has something to do with that "react-dom": "npm:@hot-loader/react-dom@16.14.0", format, as it's happening in a couple projects that all use that, which are all newly renovate enabled and all based on the same template.

I'll mess with it more.

@patrickleet
Copy link
Author

Do you just use npm i to create the lockfile? It's odd to me that my result and renovate's would be slightly different.

@rarkins
Copy link
Collaborator

rarkins commented Apr 20, 2021

It defaults to use the --package-lock-only flag, which sometimes does produce incorrect results (eg often when file: refs are used). Perhaps npm: is another scenario where it's buggy. We have logic to detect existing known buggy scenarios for npm and switch to a full node modules install instead.

@patrickleet
Copy link
Author

patrickleet commented Apr 20, 2021

That's it.

I deleted node_modules, then ran npm i --package-lock-only which reproduced the error

➜  rm -rf node_modules 
➜  npm i --package-lock-only                   
added 2200 packages in 6.937s
➜  npm ci                   
npm ERR! Cannot read property 'fetchSpec' of undefined

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/patrickleet/.npm/_logs/2021-04-20T16_55_07_686Z-debug.log

image

@rarkins
Copy link
Collaborator

rarkins commented Apr 20, 2021

Any chance if running it twice fixes it?

@patrickleet
Copy link
Author

just tried - it does not

@patrickleet patrickleet changed the title Node.js 14 / NPM 6 - invalid lockfile generated Node.js 14 / NPM 6 - Using dep with npm: prefix can result in invalid lockfiles Apr 20, 2021
@patrickleet
Copy link
Author

so basically it seems more like an npm bug

@rarkins
Copy link
Collaborator

rarkins commented Apr 20, 2021

It is, but we should be able to work around the bug like we do with file: references. See

if (skipInstalls === null) {
if (hasFileRefs) {
// https://github.com/npm/cli/issues/1432
// Explanation:
// - npm install --package-lock-only is buggy for transitive deps in file: references
// - So we set skipInstalls to false if file: refs are found *and* the user hasn't explicitly set the value already
logger.debug('Automatically setting skipInstalls to false');
skipInstalls = false;
} else {
skipInstalls = true;
}

@rarkins
Copy link
Collaborator

rarkins commented Apr 20, 2021

Can you create a minimal reproduction so that we can verify any fix works?

@patrickleet
Copy link
Author

I tried, but the error didn't happen in it - will need to spend more time on it

@rarkins
Copy link
Collaborator

rarkins commented Apr 21, 2021

private repo?

@renovate-release
Copy link
Collaborator

🎉 This issue has been resolved in version 24.119.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

@patrickleet
Copy link
Author

oh my bad - it's public now

@rarkins
Copy link
Collaborator

rarkins commented Apr 21, 2021

BTW although I submitted a hopefully automatic workaround, I realized that we allow a configurable skipInstalls which you can set to false in config, so that might have worked right away for you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto:reproduction A minimal reproduction is necessary to proceed manager:npm package.json files (npm/yarn/pnpm) status:requirements Full requirements are not yet known, so implementation should not be started type:bug Bug fix of existing functionality
Projects
None yet
Development

No branches or pull requests

3 participants