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

fix(devkit): support pnpm catalogs #30036

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

HaasStefan
Copy link
Contributor

Current Behavior

nx generate plugin throws the following error if a package like typescript is using the PNPM catalog feature in the package.json:

NX The package.json lists a version of typescript that Nx is unable to validate - (catalog:my-catalog)

"typescript": "catalog:my-catalog",

This is due too the semver validation that fails with the catalog.

Expected Behavior

If I am using a pnpm catalog, the package dependency should not be checked for a valid semver.

Related Issue(s)

Fixes #30035

@HaasStefan HaasStefan requested a review from a team as a code owner February 14, 2025 00:00
Copy link

vercel bot commented Feb 14, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Updated (UTC)
nx-dev ⬜️ Ignored (Inspect) Visit Preview Feb 14, 2025 10:08am

Copy link

nx-cloud bot commented Feb 14, 2025

View your CI Pipeline Execution ↗ for commit 608a369.

Command Status Duration Result
nx affected --targets=lint,test,build,e2e,e2e-c... ✅ Succeeded 38m 16s View ↗
nx run-many -t check-imports check-commit check... ✅ Succeeded 18s View ↗
nx-cloud record -- nx-cloud conformance:check ✅ Succeeded 2s View ↗
nx-cloud record -- nx format:check --base= --he... ✅ Succeeded 12s View ↗
nx-cloud record -- nx sync:check ✅ Succeeded 10s View ↗
nx documentation ✅ Succeeded 2m 31s View ↗

☁️ Nx Cloud last updated this comment at 2025-02-14 10:51:10 UTC

Copy link
Member

@AgentEnder AgentEnder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This util is used to get version numbers which are later used with semver helper methods, see:

const version = checkAndCleanWithSemver('tailwindcss', tailwindVersion);
if (lt(version, '2.0.0')) {
throw new Error(
`The Tailwind CSS version "${tailwindVersion}" is not supported. Please upgrade to v2.0.0 or higher.`
);
}

As such, we can't really just return the version with the catalog: protocol still there as that would result in downstream code failing as its expecting a semver.

I think to truly fix the issue you are seeing we'd need to be able to resolve the version, but I also don't love putting custom code in here for parsing pnpm-workspace.yaml and such.

@HaasStefan
Copy link
Contributor Author

@AgentEnder, what are you suggesting? Would using a parser to retrieve the pnpm version number be alright if it is not colocated in this file?

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

Successfully merging this pull request may close these issues.

PNPM Catalogs are not supported
2 participants