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

v19 gives @commitlint/config-conventional with yarn pnp #3936

Closed
1 of 4 tasks
simPod opened this issue Feb 27, 2024 · 18 comments · Fixed by #3941
Closed
1 of 4 tasks

v19 gives @commitlint/config-conventional with yarn pnp #3936

simPod opened this issue Feb 27, 2024 · 18 comments · Fixed by #3941

Comments

@simPod
Copy link

simPod commented Feb 27, 2024

Expected Behavior

No errors. Works on v18.

Current Behavior

No response

Affected packages

  • cli
  • core
  • prompt
  • config-angular

Possible Solution

No response

Steps to Reproduce

  1. use yarn pnp
  2. in package.json have
    "@commitlint/cli": "^19.0.0",
    "@commitlint/config-conventional": "^19.0.0",
  3. yarn install
  4. yarn run commitlint --from origin/$CI_DEFAULT_BRANCH --to HEAD -V
    file:///builds/build-cache/yarn/cache/@commitlint-cli-npm-19.0.0-de49d3bdb5-10c0.zip/node_modules/@commitlint/cli/lib/cli.js:131
            throw err;
            ^
    Error: Cannot find module "@commitlint/config-conventional" from "/builds/cdn/"
        at resolveId (file:///builds/build-cache/yarn/cache/@commitlint-resolve-extends-npm-19.0.0-e53f3a4be9-10c0.zip/node_modules/@commitlint/resolve-extends/lib/index.js:143:17)
        at tryResolveId (file:///builds/build-cache/yarn/cache/@commitlint-resolve-extends-npm-19.0.0-e53f3a4be9-10c0.zip/node_modules/@commitlint/resolve-extends/lib/index.js:130:12)
        at resolveConfig (file:///builds/build-cache/yarn/cache/@commitlint-resolve-extends-npm-19.0.0-e53f3a4be9-10c0.zip/node_modules/@commitlint/resolve-extends/lib/index.js:117:20)
        at file:///builds/build-cache/yarn/cache/@commitlint-resolve-extends-npm-19.0.0-e53f3a4be9-10c0.zip/node_modules/@commitlint/resolve-extends/lib/index.js:81:26
        at Array.reduce (<anonymous>)
        at loadExtends (file:///builds/build-cache/yarn/cache/@commitlint-resolve-extends-npm-19.0.0-e53f3a4be9-10c0.zip/node_modules/@commitlint/resolve-extends/lib/index.js:80:22)
        at resolveExtends (file:///builds/build-cache/yarn/cache/@commitlint-resolve-extends-npm-19.0.0-e53f3a4be9-10c0.zip/node_modules/@commitlint/resolve-extends/lib/index.js:57:28)
        at load (file:///builds/build-cache/yarn/cache/@commitlint-load-npm-19.0.0-846f918561-10c0.zip/node_modules/@commitlint/load/lib/load.js:44:28)
        at async main (file:///builds/build-cache/yarn/cache/@commitlint-cli-npm-19.0.0-de49d3bdb5-10c0.zip/node_modules/@commitlint/cli/lib/cli.js:204:20) {
      code: 'MODULE_NOT_FOUND'
    

Context

No response

commitlint --version

@commitlint/cli ^19.0.0, @commitlint/config-conventional ^19.0.0

git --version

2.41.0

node --version

v20

@JounQin
Copy link
Contributor

JounQin commented Feb 27, 2024

Can you please provide a minimal but runnable online reproduction?

We've not tested yarn P'n'P previously, AFAIK.

@chrepl
Copy link

chrepl commented Feb 27, 2024

Facing the same issue:

Just do:

npm i -g @commitlint/cli @commitlint/config-conventional
commitlint --from=$COMMIT_FROM --to=$COMMIT_TO

in the .commitlintrc.json

{ "extends": ["@commitlint/config-conventional"] }

The current workaround is to install the package @commitlint/config-conventional locally in your project, instead globally.

@JounQin
Copy link
Contributor

JounQin commented Feb 27, 2024

So you're saying global install mode, I don't know if it should be allowed. import() won't resolve global modules at all.

@simPod
Copy link
Author

simPod commented Feb 27, 2024

@chrepl I have @commitlint/config-conventional installed locally.

@JounQin
Copy link
Contributor

JounQin commented Feb 27, 2024

@chrepl I have @commitlint/config-conventional installed locally.

@simPod Please provide an online reproduction, thanks.

@MoritzWeber0
Copy link

MoritzWeber0 commented Feb 27, 2024

We are seeing the same issues in our repositories as it currently breaks all of our Github action pipelines related to commitlint.

Reproduction example (for example in a ubuntu:20.04 Docker container with npm and git installed):

npm install -g @commitlint/cli @commitlint/config-conventional
commitlint --from=$COMMIT_FROM --to=$COMMIT_TO

My .commitlintrc.yml is:

rules:
  body-leading-blank: [2, always]
  footer-leading-blank: [2, always]
  scope-case: [2, always, lower-case]
  subject-case: [2, always, sentence-case]
  subject-empty: [2, never]
  subject-full-stop: [2, never, .]
  subject-max-length: [2, always, 72]
  type-empty: [2, never]
  type-enum:
    [
      2,
      always,
      [build, chore, ci, docs, feat, fix, merge, perf, refactor, revert, test],
    ]
extends:
  - '@commitlint/config-conventional'

@MoritzWeber0
Copy link

So you're saying global install mode, I don't know if it should be allowed. import() won't resolve global modules at all.

Is there a reason why this shouldn't be allowed? I use commitlint in many repositories, so a global installation makes sense to me.

@chrepl
Copy link

chrepl commented Feb 27, 2024

@JounQin it is indeed installed globally in the CI pipeline. Using resolve-global with a custom prefix in .npmrc works fine, but not using with commitlint ....

So for example:

.npmrc

prefix = path/to/somewhere

native using 'resolve-global' package:

resolveGlobal('@commitlint/config-conventional') // works fine, could found the package

using commitlint-cli raised an error at that line.

This worked fine before v19, now this currently breaks all of our GitLab pipelines.

@simPod sorry, I spoke for myself.

@JounQin
Copy link
Contributor

JounQin commented Feb 27, 2024

resolveGlobal does not work for ESM module, so I'd expect the global install mode is a regression, but different issue from the current one, please raise a new issue instead, so that we can track it separately.

MoritzWeber0 added a commit to DSD-DBS/capella-dockerimages that referenced this issue Feb 27, 2024
Global installations don't work since the release of version v19.0.0:
conventional-changelog/commitlint#3936
MoritzWeber0 added a commit to DSD-DBS/capella-dockerimages that referenced this issue Feb 27, 2024
Global installations don't work since the release of version v19.0.0:
conventional-changelog/commitlint#3936
MoritzWeber0 added a commit to DSD-DBS/capella-collab-manager that referenced this issue Feb 27, 2024
Global installations don't work since the release of version v19.0.0:
conventional-changelog/commitlint#3936
@JounQin
Copy link
Contributor

JounQin commented Feb 27, 2024

@erik-neumann

  1. Please don't post global related issues here, raise a new issue instead
  2. Your change makes no default validation anymore, that's not a solution even workaround

@chrepl
Copy link

chrepl commented Feb 27, 2024

I'll open a new issue #3938

@DanielCastronovo DanielCastronovo mentioned this issue Feb 27, 2024
4 tasks
@escapedcat
Copy link
Member

Please check if this helps: https://github.com/conventional-changelog/commitlint/releases/tag/v19.0.1

@simPod
Copy link
Author

simPod commented Feb 27, 2024

@escapedcat I still see the same issue unfortunately.

I'll be able to prepare example repo the next week, I'm leaving tomorrow and probably will do no work.

@JounQin
Copy link
Contributor

JounQin commented Feb 28, 2024

I can reproduce it now: https://github.com/JounQin/test/tree/repro/commitlint

Root cause: wooorm/import-meta-resolve#23

We need some yarn P'n'P expert here.

cc @arcanis @merceyz @paul-soporan @larixer


See also https://yarnpkg.com/getting-started/qa#does-yarn-support-esm


Workaround PR: #3941

The remaining question is, do we need to support Yarn P'n'P officially. It's a Yarn P'n'P issue rather than commitlint's actually.

cc @escapedcat

JounQin added a commit to JounQin/commitlint that referenced this issue Feb 28, 2024
JounQin added a commit to JounQin/commitlint that referenced this issue Feb 28, 2024
escapedcat pushed a commit that referenced this issue Feb 28, 2024
* fix: fallback to `require.resolve` for Yarn P'n'P

close #3936

* refactor: use `resolve-from` package
@escapedcat
Copy link
Member

The remaining question is, do we need to support Yarn P'n'P officially. It's a Yarn P'n'P issue rather than commitlint's actually.

Good question. Thanks for your fix. I wonder how other pure ESM projects handle this. Recently lot's of projects switched. Do they all break p'n'p projects?

@escapedcat
Copy link
Member

Please try https://github.com/conventional-changelog/commitlint/releases/tag/v19.0.2

@arcanis
Copy link

arcanis commented Feb 28, 2024

I wonder how other pure ESM projects handle this. Recently lot's of projects switched. Do they all break p'n'p projects?

Yarn PnP is compatible with ESM. In this case it's a third-party package you use (import-meta-resolve) that doesn't support ESM loaders (wooorm/import-meta-resolve#10 (comment)).

@escapedcat
Copy link
Member

Thanks for clarifying @arcanis !

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

Successfully merging a pull request may close this issue.

6 participants