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

Commitlint cli running 19.0.3 gives error #3950

Open
4 tasks
akumar0212 opened this issue Mar 4, 2024 · 19 comments
Open
4 tasks

Commitlint cli running 19.0.3 gives error #3950

akumar0212 opened this issue Mar 4, 2024 · 19 comments

Comments

@akumar0212
Copy link

akumar0212 commented Mar 4, 2024

Steps to Reproduce

1. Install commitlint and set up config (ref https://commitlint.js.org/reference/configuration.html)

npm install --userconfig=./.npmrc -g commitlint-config
2. Run commitlint

this works with v18.6.1 as expected

Current Behavior

 code: 'ERR_UNKNOWN_BUILTIN_MODULE'
}
internal/process/esm_loader.js:74
    internalBinding('errors').triggerUncaughtException(
                              ^

Error [ERR_UNKNOWN_BUILTIN_MODULE]: No such built-in module: node:timers/promises
    at new NodeError (internal/errors.js:322:7)
    at Loader.builtinStrategy (internal/modules/esm/translators.js:289:11)
    at new ModuleJob (internal/modules/esm/module_job.js:63:26)
    at Loader.getModuleJob (internal/modules/esm/loader.js:258:11)
    at async ModuleWrap.<anonymous> (internal/modules/esm/module_job.js:78:21)
    at async Promise.all (index 1)
    at async link (internal/modules/esm/module_job.js:83:9) {
  code: 'ERR_UNKNOWN_BUILTIN_MODULE'
}
internal/process/esm_loader.js:74
    internalBinding('errors').triggerUncaughtException(
                              ^

Error [ERR_UNKNOWN_BUILTIN_MODULE]: No such built-in module: node:timers/promises
    at new NodeError (internal/errors.js:322:7)
    at Loader.builtinStrategy (internal/modules/esm/translators.js:289:11)
    at new ModuleJob (internal/modules/esm/module_job.js:63:26)
    at Loader.getModuleJob (internal/modules/esm/loader.js:258:11)
    at async ModuleWrap.<anonymous> (internal/modules/esm/module_job.js:78:21)
    at async Promise.all (index 1)
    at async link (internal/modules/esm/module_job.js:83:9) {
  code: 'ERR_UNKNOWN_BUILTIN_MODULE'
}
internal/process/esm_loader.js:74
    internalBinding('errors').triggerUncaughtException(

Expected Behavior

⧗   input: Merge pull request 
✔   found 0 problems, 0 warnings
⧗   input: style: formatting
✔   found 0 problems, 0 warnings
⧗   input: test: add test
✔   found 0 problems, 0 warnings
⧗   input: feat: new feat
✔   found 0 problems, 0 warnings

Affected packages

  • cli
  • core
  • prompt
  • config-angular

Possible Solution

No response

Context

No response

commitlint --version

19.0.3

git --version

2.25.1

node --version

18.x

@akumar0212 akumar0212 added the bug label Mar 4, 2024
@JounQin
Copy link
Contributor

JounQin commented Mar 8, 2024

Please provide online reproduction, not just error messages, that doesn't help.

@moran-inadvantage
Copy link

moran-inadvantage commented Mar 14, 2024

Our repo is seeing this issue as well on the latest release. Hopefully this is what you mean by online repo.

@JounQin

I have created a public repo that shows the issue:
https://github.com/moran-inadvantage/commit_lint_issue_3950

Here is an example of the failure:
https://github.com/moran-inadvantage/commit_lint_issue_3950/actions/runs/8287030551/job/22678414864

You can trigger a workflow that fails here:
https://github.com/moran-inadvantage/commit_lint_issue_3950/actions/workflows/commit-validate.yml

@JounQin
Copy link
Contributor

JounQin commented Mar 14, 2024

@moran-inadvantag I don't think we support legacy node v14, node v18+ is required.

@knocte
Copy link
Contributor

knocte commented Mar 23, 2024

node v18+ is required.

Maybe we should detect the version before running, and fail fast in case it's too old?

@knocte
Copy link
Contributor

knocte commented Mar 23, 2024

Maybe we should detect the version before running, and fail fast in case it's too old?

Something like this? https://gist.github.com/knocte/f78f8f60800e54bce59110138389105b

@knocte
Copy link
Contributor

knocte commented Apr 8, 2024

Maybe we should detect the version before running, and fail fast in case it's too old?

Something like this? https://gist.github.com/knocte/f78f8f60800e54bce59110138389105b

@escapedcat @JounQin what do you guys think?

@JounQin
Copy link
Contributor

JounQin commented Apr 8, 2024

Hmm... Didn't engines field warning about the incompatible usage? I'm not sure whether we need to check it another time.

@knocte
Copy link
Contributor

knocte commented Apr 8, 2024

Hmm... Didn't engines field warning about the incompatible usage? I'm not sure whether we need to check it another time.

If that was the case, wouldn't @moran-inadvantage have got a different error?

@JounQin
Copy link
Contributor

JounQin commented Apr 8, 2024

The warning is occurred on installation AFAIK.

@escapedcat
Copy link
Member

This is the error you get when you install it with a wrong version:

image
 user@machine ~/test npm install --save-dev @commitlint/{cli,config-conventional}
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@commitlint/cli@19.2.1',
npm WARN EBADENGINE   required: { node: '>=v18' },
npm WARN EBADENGINE   current: { node: 'v16.14.0', npm: '8.3.1' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@commitlint/config-conventional@19.1.0',
npm WARN EBADENGINE   required: { node: '>=v18' },
npm WARN EBADENGINE   current: { node: 'v16.14.0', npm: '8.3.1' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@commitlint/format@19.0.3',
npm WARN EBADENGINE   required: { node: '>=v18' },
npm WARN EBADENGINE   current: { node: 'v16.14.0', npm: '8.3.1' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@commitlint/lint@19.1.0',
npm WARN EBADENGINE   required: { node: '>=v18' },
npm WARN EBADENGINE   current: { node: 'v16.14.0', npm: '8.3.1' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@commitlint/load@19.2.0',
npm WARN EBADENGINE   required: { node: '>=v18' },
npm WARN EBADENGINE   current: { node: 'v16.14.0', npm: '8.3.1' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@commitlint/read@19.2.1',
npm WARN EBADENGINE   required: { node: '>=v18' },
npm WARN EBADENGINE   current: { node: 'v16.14.0', npm: '8.3.1' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@commitlint/types@19.0.3',
npm WARN EBADENGINE   required: { node: '>=v18' },
npm WARN EBADENGINE   current: { node: 'v16.14.0', npm: '8.3.1' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: 'execa@8.0.1',
npm WARN EBADENGINE   required: { node: '>=16.17' },
npm WARN EBADENGINE   current: { node: 'v16.14.0', npm: '8.3.1' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@commitlint/is-ignored@19.0.3',
npm WARN EBADENGINE   required: { node: '>=v18' },
npm WARN EBADENGINE   current: { node: 'v16.14.0', npm: '8.3.1' }
npm WARN EBADENGINE }
...

I think this should be enough, no?

@knocte
Copy link
Contributor

knocte commented Apr 8, 2024

So @moran-inadvantage didn't get this because he used it directly via npx instead of installing?

@escapedcat

This comment was marked as outdated.

@knocte
Copy link
Contributor

knocte commented Apr 8, 2024

So we need the "Unsupported engine" checks at runtime too, not just installation time.

@escapedcat
Copy link
Member

Ah no, same. Forgot to delete the node_modules in my test folder...

user@machine ~/test npx @commitlint/cli
Need to install the following packages:
  @commitlint/cli
Ok to proceed? (y) y
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@commitlint/cli@19.2.1',
npm WARN EBADENGINE   required: { node: '>=v18' },
npm WARN EBADENGINE   current: { node: 'v16.14.0', npm: '8.3.1' }
npm WARN EBADENGINE }

@knocte
Copy link
Contributor

knocte commented Apr 10, 2024

So why did @moran-inadvantage not get the above, @escapedcat ?

@JounQin
Copy link
Contributor

JounQin commented Apr 10, 2024

image

There is a warning, but the user didn't notice that I think.

https://github.com/moran-inadvantage/commit_lint_issue_3950/actions/runs/8287030551/job/22678414864#step:4:9

@knocte
Copy link
Contributor

knocte commented Apr 10, 2024

So, should/can we convert that warning into an error?

@escapedcat
Copy link
Member

Hm, can't decide.
On the one hand the warning seems to be the default in the ecosystem and people should be used to that?
On the other hand it might be nice to give a clear error.

Apparently other packages are also using different strategies:

npx prettier
prettier requires at least version 14 of Node, please upgrade
npx eslint

Oops! Something went wrong! :(

ESLint: 8.56.0

TypeError: Module.createRequire is not a function
    at Object.<anonymous> (/test/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2404:26)
    at Module._compile (internal/modules/cjs/loader.js:778:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)
    at Module.load (internal/modules/cjs/loader.js:653:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:593:12)
    at Function.Module._load (internal/modules/cjs/loader.js:585:3)
    at Module.require (internal/modules/cjs/loader.js:692:17)
    at require (internal/modules/cjs/helpers.js:25:18)
    at Object.<anonymous> (/test/node_modules/eslint/lib/cli-engine/cli-engine.js:33:5)
    at Module._compile (internal/modules/cjs/loader.js:778:30)

@knocte
Copy link
Contributor

knocte commented Apr 10, 2024

The ideal would be the best of both worlds:

  • By default, crash with an error saying that version of Node is unsupported, but:
  • Ask user to upgrade or use some flag to run at your own risk, e.g. '--ignore-minimum-required-dependencies' (this flag would convert the error into just a warning).

At least this way people would not report bugs that are related to this, and would try to upgrade first.

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

No branches or pull requests

5 participants