search

conventional-changelog / commitlint

📓 Lint commit messages
https://commitlint.js.org
MIT License
16.89k stars 909 forks source link

Commitlint cli running 19.0.3 gives error #3950

Open akumar0212 opened 8 months ago

akumar0212 commented 8 months ago

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

Possible Solution

No response

Context

No response

commitlint --version

19.0.3

git --version

2.25.1

node --version

18.x

JounQin commented 8 months ago

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

moran-inadvantage commented 8 months ago

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 commented 8 months ago

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

knocte commented 7 months ago

node v18+ is required.

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

knocte commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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

knocte commented 7 months ago

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 commented 7 months ago

The warning is occurred on installation AFAIK.

escapedcat commented 7 months ago

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 commented 7 months ago

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

escapedcat commented 7 months ago

Yeah, that could be the case:

user@machine ~/test npx @commitlint/cli
@commitlint/cli@19.2.1 - Lint your commit messages

[input] reads from stdin if --edit, --env, --from and --to are omitted

Options:
  -c, --color          toggle colored output                                                                                                                                                                                                       [boolean] [default: true]
  -g, --config         path to the config file
...
knocte commented 7 months ago

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

escapedcat commented 7 months ago

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 commented 7 months ago

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

JounQin commented 7 months ago
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 commented 7 months ago

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

escapedcat commented 7 months ago

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 commented 7 months ago

The ideal would be the best of both worlds:

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

lishaduck commented 4 months ago

There's also "enginesStrict": true, for when your code will definitely not work on old versions, like this. If, i.e., I dropped an old Node becuase it was old, you wouldn't set that, but if I dropped it becuase it doesn't work, then you would.