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:" is not supported #204

Open
ritz078 opened this issue Aug 28, 2024 · 10 comments
Open

pnpm: "catalog:" is not supported #204

ritz078 opened this issue Aug 28, 2024 · 10 comments

Comments

@ritz078
Copy link

ritz078 commented Aug 28, 2024

The catalog: protocol is a new feature that is not supported by this app yet. https://pnpm.io/catalogs

@Aslemammad
Copy link
Member

Thank you so much for reporting this!

have you tried --pnpm flag? Would it fix the issue?

@ritz078
Copy link
Author

ritz078 commented Aug 28, 2024

I used pnpx pkg-pr-new --compact --pnpm publish './packages/*' but that didn't work. It's working fine for workspace protocol.

@Aslemammad
Copy link
Member

I will have a look, thanks!

@Aslemammad
Copy link
Member

Hmm, do you have any reproduction or a repo?

I want to see what kind of workspace pkg.pr.new should adapt to.

@ritz078
Copy link
Author

ritz078 commented Sep 3, 2024

The code is not public but here's how the relevant files look.

pnpm-workspace.yaml

packages:
  - "packages/*"
  - "apps/*"

catalog:
  "@react-aria/utils": "3.24.1"
  "@react-aria/interactions": "3.21.3"

packages/core

{
  "name": "",
  "version": "0.28.0",
  "description": "",
  "main": "dist/index.js",
  "module": "dist/index.mjs",
  "types": "dist/index.d.ts",
  "style": "dist/index.css",
  "dependencies": {
    "@baseline-ui/core": "workspace:*",
    "@baseline-ui/utils": "workspace:*",
    "@react-aria/interactions": "catalog:",
    "@react-aria/utils": "catalog:",
    "@react-spring/web": "^9.7.4",
  },
  "peerDependencies": {
    "react": "^17.0.2 || ^18.0.0",
    "react-dom": "^17.0.2 || ^18.0.0"
  },
}

I was able to resolve this by running a script that replaces all occurences of catalog: by their specific versions from pnpm-workspace.yaml

#!/usr/bin/env ts-node

import { pick } from "lodash";
import fs from "node:fs/promises";
import path from "node:path";
import yaml from "yaml";

// This script removes the catalog from the dependencies of the packages in the monorepo.
// Temporary workaround for https://github.com/stackblitz-labs/pkg.pr.new/issues/204
void (async function () {
  const pnpmWorkspace = await fs.readFile(
    path.resolve(__dirname, "./pnpm-workspace.yaml"),
  );

  const catalog = yaml.parse(pnpmWorkspace.toString("utf8")).catalog;

  const packagesName = await fs.readdir(path.resolve(__dirname, "./packages"));

  const packages = packagesName.map((dir) =>
    path.resolve(__dirname, `./packages/${dir}`),
  );

  for (const package_ of packages) {
    const packageJson = await fs.readFile(
      path.resolve(package_, "package.json"),
    );

    const packageJsonObj = JSON.parse(packageJson.toString("utf8"));

    const dependencies = packageJsonObj.dependencies || {};
    packageJsonObj.dependencies = {
      ...dependencies,
      ...pick(catalog, Object.keys(dependencies)),
    };

    const devDependencies = packageJsonObj.devDependencies || {};
    packageJsonObj.devDependencies = {
      ...devDependencies,
      ...pick(catalog, Object.keys(devDependencies)),
    };

    await fs.writeFile(
      path.resolve(package_, "package.json"),
      JSON.stringify(packageJsonObj, null, 2),
    );
  }
})();

@jacob-8
Copy link

jacob-8 commented Sep 6, 2024

I can confirm this is also an issue in UnoCSS - see unocss/unocss#4122 for description and reproduction.

We are also using the --pnpm (see here) and still have the issue.

@Aslemammad
Copy link
Member

Interesting, now I'm thinking, how does pnpm handles catalogs?

Does it handle catalogs in pnpm publish? If yes why it does not handle that in pnpm pack too?

Since pkg.pr.new uses pnpm pack, I wonder what should we do to support catalogs in that case?

cc @zkochan out of curiosity!

@jacob-8
Copy link

jacob-8 commented Sep 6, 2024

Interesting, now I'm thinking, how does pnpm handles catalogs?

Does it handle catalogs in pnpm publish? If yes why it does not handle that in pnpm pack too?

The docs indicate both pnpm publish and pnpm pack both handle it which is why this is all a little odd.

@Aslemammad
Copy link
Member

Aslemammad commented Sep 6, 2024

The docs indicate both pnpm publish and pnpm pack both handle it which is why this is all a little odd.

Oh nice catch, yea, since we use pnpm pack, it should be handled but there might be a bug there?

I think it'd be good to reproduce without pkg.pr.new and see if it's an actual bug in pnpm itself.

SokratisVidros added a commit to novuhq/novu that referenced this issue Sep 19, 2024
Based on the following comment, using `pnpm` should resolve the workspace:*protocol correctly. Let's try it.

stackblitz-labs/pkg.pr.new#204 (comment)
@SokratisVidros
Copy link

@ritz078 can you please confirm that using --pnpm works for workspace:* protocol? I've just tried it here and the package.json of the package still contains workspace:*.

We also have an in-house patch to replace all workspace:* occurrences with latest before publishing. This prevents us from doing changes in multiple packages in the same PR, especially if the changes in one package are used in the other as we can't link the preview packages.

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

No branches or pull requests

4 participants