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

pnpm catalog fails to parse version in package.json #1965

Open
yutakobayashidev opened this issue Mar 10, 2025 · 12 comments · May be fixed by #1989
Open

pnpm catalog fails to parse version in package.json #1965

yutakobayashidev opened this issue Mar 10, 2025 · 12 comments · May be fixed by #1989
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@yutakobayashidev
Copy link

When using Orval with the catalog feature of pnpm workspace, an error occurs because it cannot parse the version in package.json.

{
  "name": "test",
  "dependencies": {
    "@tanstack/react-query": "catalog:"
  }
}
packages:
  - packages/*

catalog:
  "@tanstack/react-query": "^5.67.2"
~/pnpm-test/packages/test$ pnpm orval
🍻 Start orval v7.6.0 - A swagger client generator for typescript
🛑 petstore - Error: Invalid argument not valid semver ('catalog:' received)

I tried to modify the code so that it parses the pnpm-workspace.yaml, extracts the catalog version, and maps it to package.json as follows. What do you think? There might be adjustments needed, like handling cases where a name is not found in the catalog and retaining the original 'catalog:'. If everything looks good, I can create a pull request.

interface PnpmWorkspace {
  packages?: string[];
  catalog?: Record<string, string>;
}

const loadPnpmWorkspace = async (
  workspace = process.cwd(),
): Promise<PnpmWorkspace | undefined> => {
  const workspacePath = await findUp(['pnpm-workspace.yaml'], {
    cwd: workspace,
  });

  if (!workspacePath) {
    return undefined;
  }

  try {
    const content = await fs.readFile(workspacePath, 'utf8');
    return yaml.load(content) as PnpmWorkspace;
  } catch {
    return;
  }
};

const replaceCatalogReferences = (
  pkg: PackageJson,
  catalog: Record<string, string>,
): PackageJson => {
  const replaceDependencies = (
    deps?: Record<string, string>,
  ): Record<string, string> | undefined => {
    if (!deps) return undefined;

    const newDeps: Record<string, string> = {};
    for (const [name, version] of Object.entries(deps)) {
      if (typeof version === 'string' && version === 'catalog:') {
        newDeps[name] = catalog[name] || version;
      } else {
        newDeps[name] = version;
      }
    }
    return newDeps;
  };

  return {
    ...pkg,
    dependencies: replaceDependencies(pkg.dependencies),
    devDependencies: replaceDependencies(pkg.devDependencies),
    peerDependencies: replaceDependencies(pkg.peerDependencies),
  };
};

export const loadPackageJson = async (
  packageJson?: string,
  workspace = process.cwd(),
): Promise<PackageJson | undefined> => {

  const pkgPath = packageJson
    ? normalizePath(packageJson, workspace)
    : await findUp(['package.json'], { cwd: workspace });

  if (!pkgPath || !fs.existsSync(pkgPath)) {
    return;
  }

  const pkg = await import(pkgPath);

  const pnpmWorkspace = await loadPnpmWorkspace(workspace);
  const catalog = pnpmWorkspace?.catalog;

  if (!catalog) {
    return pkg;
  }

  return replaceCatalogReferences(pkg, catalog);
};
@yutakobayashidev
Copy link
Author

duplicate #1931

@yutakobayashidev
Copy link
Author

yutakobayashidev commented Mar 10, 2025

I used version 7.6.0, but the issue persists. The catalog is used in multiple dependencies.

🛑 petstore - Oups... 🍻. An Error occurred while writing file => Error: Invalid argument not valid semver ('catalog:' received)

@melloware
Copy link
Collaborator

@shotanue can you confirm?

@melloware melloware added the bug Something isn't working label Mar 10, 2025
@shotanue
Copy link

7.6.0 seems to have fixed the issue I reported in #1931.
@melloware, Thank you for addressing it.

Here's my orval.config.ts that I'm using:

import { defineConfig } from "orval";
export default defineConfig({
  xxxxx: {
    input: "xxxxx/openapi.yaml",
    output: {
      mode: "split",
      client: "react-query",
      target: "xxxxxx",
      override: {
        query: {
          useSuspenseQuery: true,
+          version: 5,
        },
        mutator: {
          path: "./src/custom-instance.ts",
          name: "customInstance",
        },
      },
    },
  },
});

When using pnpm catalogs, it appears that specifying override.query.version = 5 is necessary.
I tried removing this option in 7.6.0, and it resulted in a "not valid semver" error.
It functions as a workaround for errors related to the pnpm catalogs protocol.
This may differ from the option's original purpose, though. 🤔

@melloware melloware added this to the 7.6.0 milestone Mar 10, 2025
@melloware melloware added the workaround A workaround has been provided label Mar 10, 2025
@yutakobayashidev
Copy link
Author

yutakobayashidev commented Mar 10, 2025

I tried that workaround, but when there are other dependencies managed by the catalog, a slightly different error occurs. In #1931, it was not an error related to file writing. The issue is resolved and works fine by applying a patch that resolves the catalog version mentioned in the first comment.

🛑 petstore - Oups... 🍻. An Error occurred while writing file => Error: Invalid argument not valid semver ('catalog:' received)

@melloware
Copy link
Collaborator

@shotanue any thoughts?

@shotanue
Copy link

I apologize for not reading the issue body thoroughly.
This issue was actually a proposal for catalog support.

I did not come up with alternative workarounds.

As an Orval user, it would be better if Orval supported pnpm catalogs due to the current limitations that sometimes require workarounds, which don't always work reliably.

The proposal to support pnpm catalogs with sanitizing package.json is a good idea 👍 If it supported https://pnpm.io/catalogs#named-catalogs, it would make for an even better implementation.

The proposed solution could be applicable beyond react-query, eliminating the need for workarounds, which would be a great improvement for Orval.

@melloware melloware reopened this Mar 11, 2025
@melloware
Copy link
Collaborator

Reopened. I know nothing about PNPM and catalog so this will most likely require a PR from a user...

@melloware melloware added enhancement New feature or request and removed bug Something isn't working workaround A workaround has been provided labels Mar 11, 2025
@arthurfiorette
Copy link
Contributor

Puting packageJson: 'fake', in options supresses this issue.suppresses

@melloware melloware removed this from the 7.6.0 milestone Mar 13, 2025
@melloware melloware added the good first issue Good for newcomers label Mar 13, 2025
@fernion
Copy link

fernion commented Mar 24, 2025

Just to add that on version 7.7.0 of Orval, in a monorepo using pnpm and catalogs, if the typescript dependency is specified as "catalog:" then it fails with the error:
Oups... 🍻. An Error occurred while splitting => Error: Invalid argument not valid semver ('catalog:' received)

Seems a related issue to this.

Puting packageJson: 'fake', in options supresses this issue.suppresses

This workaround circumvents the error, but the generated code is a lot different.

@AllieJonsson AllieJonsson linked a pull request Mar 25, 2025 that will close this issue
@AllieJonsson
Copy link
Contributor

I have added a fix for this, but as I dont use pnpm myself I have a few questions:

  1. Does pnpm fall back to using the default catalog if a package is not defined in a named catalog?
  2. Do we need an option for specifying where your pnpm-workspace.yaml is located? Afaict from the docs it should always be in the root of your project?

@fernion
Copy link

fernion commented Mar 26, 2025

  1. Does pnpm fall back to using the default catalog if a package is not defined in a named catalog?
    No, it will fail with an error No catalog entry 'some-package-name' was found for catalog 'customcatalog'.
  1. Do we need an option for specifying where your pnpm-workspace.yaml is located? Afaict from the docs it should always be in the root of your project?
    I believe this is correct (from the docs and from the usages I have seen).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants