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/workspace.pkgs-graph] Ignore // key in dependencies #7513

Closed
2 of 4 tasks
alexeyr-ci opened this issue Jan 10, 2024 · 4 comments
Closed
2 of 4 tasks

[@pnpm/workspace.pkgs-graph] Ignore // key in dependencies #7513

alexeyr-ci opened this issue Jan 10, 2024 · 4 comments

Comments

@alexeyr-ci
Copy link

alexeyr-ci commented Jan 10, 2024

Verify latest release

  • I verified that the issue exists in the latest pnpm release

pnpm version

No response

Which area(s) of pnpm are affected? (leave empty if unsure)

Dependencies resolver

Link to the code that reproduces this issue or a replay of the bug

No response

Reproduction steps

package.json:

{
  "name": "pnpm-workspace.pkgs-graph-comment-repro",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "keywords": [],
  "author": "",
  "license": "ISC",
  "dependencies": {
  },
  "devDependencies": {
    "//": ["a comment"],
    "@pnpm/workspace.pkgs-graph": "^2.0.13"
  }
}

script demo.js:

const { createPkgGraph } = require('@pnpm/workspace.pkgs-graph');

const manifest = require('./package.json');

const workspaceManifest = { dir: __dirname, manifest };

console.log(createPkgGraph([workspaceManifest]));

Run the script.

Describe the Bug

It fails with this error:

/home/alexey/shaka/pnpm-workspace.pkgs-graph-comment-repro/node_modules/@pnpm/workspace.pkgs-graph/lib/index.js:32
            const isWorkspaceSpec = rawSpec.startsWith('workspace:');
                                            ^

TypeError: rawSpec.startsWith is not a function
    at /home/alexey/shaka/pnpm-workspace.pkgs-graph-comment-repro/node_modules/@pnpm/workspace.pkgs-graph/lib/index.js:32:45
    at Array.map (<anonymous>)
    at createNode (/home/alexey/shaka/pnpm-workspace.pkgs-graph-comment-repro/node_modules/@pnpm/workspace.pkgs-graph/lib/index.js:30:14)
    at /home/alexey/shaka/pnpm-workspace.pkgs-graph-comment-repro/node_modules/@pnpm/workspace.pkgs-graph/lib/index.js:19:23
    at XWrap.f (/home/alexey/shaka/pnpm-workspace.pkgs-graph-comment-repro/node_modules/ramda/src/map.js:78:20)
    at XWrap.@@transducer/step (/home/alexey/shaka/pnpm-workspace.pkgs-graph-comment-repro/node_modules/ramda/src/internal/_xwrap.js:17:17)
    at _arrayReduce (/home/alexey/shaka/pnpm-workspace.pkgs-graph-comment-repro/node_modules/ramda/src/internal/_reduce.js:18:34)
    at _reduce (/home/alexey/shaka/pnpm-workspace.pkgs-graph-comment-repro/node_modules/ramda/src/internal/_reduce.js:60:12)
    at map (/home/alexey/shaka/pnpm-workspace.pkgs-graph-comment-repro/node_modules/ramda/src/map.js:77:14)
    at /home/alexey/shaka/pnpm-workspace.pkgs-graph-comment-repro/node_modules/ramda/src/internal/_dispatchable.js:50:15

Node.js v18.17.0

Expected Behavior

It should succeed. The "//" key is the recommended way to add "comments" to package.json, and making the value an array is the way to preserve multiple comments.

Which Node.js version are you using?

18.17.0

Which operating systems have you used?

  • macOS
  • Windows
  • Linux

If your OS is a Linux based, which one it is? (Include the version if relevant)

Ubuntu (on WSL)

Related issue: #6618

@zkochan
Copy link
Member

zkochan commented Jan 11, 2024

It should succeed. The "//" key is the recommended way to add "comments" to package.json, and making the value an array is the way to preserve multiple comments.

Where do you get this information? npm fails too if you put "//" into the dependencies. This is not supported.

pnpm works with package.yaml, where you can use comments. But it is not recommended to use anything else besides package.json.

@alexeyr-ci
Copy link
Author

alexeyr-ci commented Jan 11, 2024

https://groups.google.com/g/nodejs/c/NmL7jdeuw0M/m/yTqI05DRQrIJ

That being said, I completely understand the desire to put additional
data into your JSON configuration files. That's why the "//" key will
never be used by npm for any purpose, and is reserved for comments.

If you want to use a multiple line comment, you can use either an
array, or multiple "//" keys.

Of course, that's from 2013... The problem is, it does work in yarn, and I am not using pnpm myself; I wanted to use Knip which happens to use @pnpm/workspace.pkgs-graph as a dependency on my yarn project.

@zkochan
Copy link
Member

zkochan commented Jan 11, 2024

So if your project uses Yarn, then why is Knip using a pnpm package?

@webpro
Copy link

webpro commented Jan 14, 2024

So if your project uses Yarn, then why is Knip using a pnpm package?

Fwiw, Knip is a project linter and uses @pnpm/workspace.pkgs-graph internally to find dependents of a workspace. This essentially has nothing to do with the package manager for a given repo, but occasionally might run into issues like this.

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

No branches or pull requests

3 participants