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

Skipping checks for Yarn / npm workspaces #1217

Closed
3 tasks done
kachkaev opened this issue Nov 9, 2022 · 6 comments
Closed
3 tasks done

Skipping checks for Yarn / npm workspaces #1217

kachkaev opened this issue Nov 9, 2022 · 6 comments
Labels

Comments

@kachkaev
Copy link

kachkaev commented Nov 9, 2022

  • I have searched for similar issues
  • I am using the latest version of npm-check-updates (16.3.18)
  • I am using node >= 14.14

👋 @raineorshine! When I run ncu in a monorepo with --workspaces (or --deep) option, I see warnings like this:

404 Not Found - GET https://registry.npmjs.org/some-local-package. Either your internet connection is down or unstable and all 3 retry attempts failed, or the registry is not accessible, or the package does not exist. 

When ncu scans workspace package.json files, it does not distinguish between the references to third-party npm packages and local workspaces, which are sourced from the sibling folders. It’d be nice if ncu skipped workspace references by default.

Steps to Reproduce

You can try running ncu --workspaces in one of monorepo examples: https://github.com/vercel/turbo/tree/main/examples. Alternatively, you can clone https://github.com/blockprotocol/blockprotocol and run ncu from the root.

You can also use this MWE:

mkdir /tmp/ncu-1217
cd /tmp/ncu-1217

cat << EOF > package.json
{
  "private": true,
  "workspaces": [
    "packages/*"
  ]
}
EOF


mkdir -p packages/some-local-package packages/some-local-app

cat << EOF > packages/some-local-package/package.json
{
  "name": "some-local-package",
  "version": "0.4.2",
  "dependencies": {
    "eslint": "1.0.0"
  }
}
EOF

cat << EOF > packages/some-local-app/package.json
{
  "name": "some-local-app",
  "dependencies": {
    "some-local-package": "0.4.2",
    "eslint": "1.0.0"
  }
}
EOF

npx npm-check-updates@16.3.18 --workspaces

Current Behavior

References to other workspaces are checked.

In the MWE:

Checking /private/tmp/example-repo/packages/some-local-app/package.json
[==========----------] 1/2 50%
 eslint  1.0.0  →  8.27.0

 some-local-package  404 Not Found - GET https://registry.npmjs.org/some-local-package. Either your internet connection is down or unstable and all 3 retry attempts failed, or the registry is not accessible, or the package does not exist. 

Run npx npm-check-updates --workspaces -u to upgrade packages/some-local-app/package.json
Checking /private/tmp/example-repo/packages/some-local-package/package.json
[====================] 1/1 100%

 eslint  1.0.0  →  8.27.0

Expected Behavior

References to other workspaces are ignored.

In the MWE:

Checking /private/tmp/example-repo/packages/some-local-app/package.json
[==========----------] 1/1 100%
 eslint  1.0.0  →  8.27.0

Run npx npm-check-updates --workspaces -u to upgrade packages/some-local-app/package.json
Checking /private/tmp/example-repo/packages/some-local-package/package.json
[====================] 1/1 100%

 eslint  1.0.0  →  8.27.0

Additional thoughts

I think it’s fine to keep the current behavior when running ncu --deep because in this case we are not necessarily running ncu in a monorepo. I can also imagine some option that distinguishes between links to workspaces with "prviate": true and those that actually get published, but I struggle to come up with an example where different behaviour would be needed. Just passing the full list of workspaces to the ignore list should be a good start IMHO.

@kachkaev kachkaev changed the title Skipping private workspaces Skipping checks for yarn / npm workspaces Nov 9, 2022
@kachkaev kachkaev changed the title Skipping checks for yarn / npm workspaces Skipping checks for Yarn / npm workspaces Nov 9, 2022
@raineorshine
Copy link
Owner

In addition to the general description in current and expected behavior, can you provide a concrete description of the particular current and expected behavior of one your test cases? Just to make sure I'm on the same page. Thanks!

@kachkaev
Copy link
Author

kachkaev commented Nov 9, 2022

Done. Happy to provide more info if needed.

@raineorshine
Copy link
Owner

Fixed and published in v16.3.21

@kachkaev
Copy link
Author

kachkaev commented Nov 11, 2022

Thanks @raineorshine! f13fd29 makes sense and it should cover many cases. In the two monorepo I work in package names do not strictly match folder names so I still get warnings. Basically this line does not work for me:

workspacePackages = workspacePackageFiles.map(file => file.split('/').slice(-2)[0])

What ncu can do to address this is reading package.json files and extracting workspace names from them. Alternatively we can get the names from yarn workspaces info if available.

Examples:

@raineorshine
Copy link
Owner

Ah, I see. I hadn't thought of that.

Should be fixed up and published in v16.3.22.

@kachkaev
Copy link
Author

v16.3.22 works like a charm ✨ Big thanks for fixing this bug so quickly and happy Friday 🙌 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants