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

Broken npm ci since forks replaced official dependencies #5929

Closed
aubm opened this issue Sep 7, 2021 · 9 comments · Fixed by #6004
Closed

Broken npm ci since forks replaced official dependencies #5929

aubm opened this issue Sep 7, 2021 · 9 comments · Fixed by #6004
Milestone

Comments

@aubm
Copy link

aubm commented Sep 7, 2021

Hello,

It appears that recent changes in package.json have broken the npm ci with npm < v7.x

Probably due to changes to the lockfile.

Prior to npm 7 yarn.lock files were ignored

Source: https://github.blog/2020-10-13-presenting-v7-0-0-of-the-npm-cli/

Steps to reproduce the error with npm 6

cat '{
  "name": "test-project",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC",
  "dependencies": {
    "plotly.js": "^2.5.0"
  }
}' > package.json

npm install --package-lock-only
npm ci

npm ci exists with the following error:

npm ERR! code ETARGET
npm ERR! notarget No matching version found for [email protected].
npm ERR! notarget In most cases you or one of your dependencies are requesting
npm ERR! notarget a package version that doesn't exist.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/aurelienbaumann/.npm/_logs/2021-09-07T20_14_08_058Z-debug.log

Looking in package-lock.json to see the details about zero-crossings, I can see the resolved field is ignored.

"zero-crossings": {
  "version": "1.1.0",
  "resolved": "git+https://github.com/plotly/zero-crossings.git#4746771f490344e21c84d1f631cb661d8c5ff508"
}

We should probably find a workaround this if we want to keep supporting npm < 7.

@nicolaskruchten
Copy link
Contributor

We're looking into this! In general, we recommend that folks depend upon a prebuilt bundle like plotly.js-dist or create a custom bundle and include that in their source tree, rather than depending upon plotly.js directly. We'll try to fix this, but we don't rigorously test this case so we may break it again in the future.

@aubm
Copy link
Author

aubm commented Sep 8, 2021

Thank you @nicolaskruchten 🙂

@archmoj
Copy link
Contributor

archmoj commented Sep 8, 2021

Thank you @nicolaskruchten

@aubm are you able to use plotly.js-dist or plotly.js-dist-min in your project?

@aubm
Copy link
Author

aubm commented Sep 8, 2021

@archmoj just to be clear, that would mean not using npm, right?

@nicolaskruchten
Copy link
Contributor

@aubm
Copy link
Author

aubm commented Sep 8, 2021

Oh perfect thank you 👌 that should do it

@archmoj
Copy link
Contributor

archmoj commented Sep 8, 2021

@archmoj just to be clear, that would mean not using npm, right?

No. dist packages are available on npm e.g.
https://www.npmjs.com/package/plotly.js-dist
https://www.npmjs.com/package/plotly.js-dist-min

You simply need to uninstall plotly.js and install plotly.js-dist or plotly.js-dist-min.

However if you have react-plotly.js in your dependencies, then you need to wait for plotly/react-plotly.js#260 or alternatively link to commits from that PR.

@aubm
Copy link
Author

aubm commented Sep 8, 2021

Noted, thank you 🙂

@sepans
Copy link

sepans commented Sep 29, 2021

We are having the same issue when trying to build the package on a build/ci machine that doesn't have open access to the internet (e.g. github). I used plotly.js-dist and bundle customization referenced in plotly/react-plotly.js#260 and now the builds are working.

@archmoj archmoj added this to the v2.6.0 milestone Oct 29, 2021
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 a pull request may close this issue.

4 participants