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

enginesNode should not filter out packages with no engine field #1313

Closed
3 tasks done
regevbr opened this issue Aug 14, 2023 · 3 comments
Closed
3 tasks done

enginesNode should not filter out packages with no engine field #1313

regevbr opened this issue Aug 14, 2023 · 3 comments
Labels

Comments

@regevbr
Copy link
Contributor

regevbr commented Aug 14, 2023

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

Currently, when setting the enginesNode option, and setting the node version on our package.json file, any package that does not specify its node version at all, will be ignored.
I think we should allow current behaviour, maybe as enginesNodeStrict and make enginesNode be more relaxed, by not skipping packages that don't specify the node version.

The filtering happens here:
https://github.com/raineorshine/npm-check-updates/blob/3bf214997322371fbb8cafa168186bb04b900435/src/package-managers/filters.ts#L38C17-L44

export function satisfiesNodeEngine(versionResult: Packument, nodeEngineVersion: Maybe<string>): boolean {
  if (!nodeEngineVersion) return true
  const minVersion = get(semver.minVersion(nodeEngineVersion), 'version')
  if (!minVersion) return true
  const versionNodeEngine: string | undefined = get(versionResult, 'engines.node')
  return !!versionNodeEngine && semver.satisfies(minVersion, versionNodeEngine)
}

Steps to Reproduce

.ncurc:

Dependencies:

{
  "name": "test",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "engines": {
    "node": ">= 20.5.1"
  },
  "engineStrict": true,
  "dependencies": {
    "@types/express": "^4.0.29",
    "npm-check-updates": "^16.11.1"
  }
} 

Steps:

  • ncu --target greatest --pre 1 --enginesNode

Current Behavior

no upgrade is suggested

Expected Behavior

upgrade to 4.7.17 should be suggested

@raineorshine
Copy link
Owner

raineorshine commented Aug 14, 2023

Are you sure you are using the latest version of npm-check-updates? --greatest was deprecated a while ago.

The given command works as expected on npm-check-updates v16.11.1.

@regevbr
Copy link
Contributor Author

regevbr commented Aug 15, 2023

Are you sure you are using the latest version of npm-check-updates? --greatest was deprecated a while ago.

The given command works as expected on npm-check-updates v16.11.1.

Type on my end, sorry. Yes I'm using the latest version.
I have updated with a full package json that reproduces the issue. The key is the engines restriction. Can you please have another look?

@raineorshine
Copy link
Owner

Thank you. I'm not sure if the original implementation in #602 was flawed, or if this was a later regression, but it is definitely not working correctly for packages without an engines.node entry.

The tests may have subtly changed behavior too as they are querying the live registry, which changes over time. I updated the relevant test to use https://github.com/raineorshine/ncu-test-v2 so there is no more slippage.

Fixed and published in v16.11.2.

@regevbr regevbr closed this as completed Aug 16, 2023
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