Ignore "//" key in (dev)depedencies

[alexeyr-ci]

When I run knip 3.13.0 on my project, I get

alexey@DESKTOP-41AVESA:~/shaka/popmenu$ knip                                             
Reading workspace configuration(s)...
/home/alexey/shaka/popmenu/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/popmenu/node_modules/@pnpm/workspace.pkgs-graph/lib/index.js:32:45
    at Array.map (<anonymous>)                                                                           
    at createNode (/home/alexey/shaka/popmenu/node_modules/@pnpm/workspace.pkgs-graph/lib/index.js:30:14)
    at /home/alexey/shaka/popmenu/node_modules/@pnpm/workspace.pkgs-graph/lib/index.js:19:23
    at XWrap.f (/home/alexey/shaka/popmenu/node_modules/ramda/src/map.js:78:20)                            
    at XWrap.@@transducer/step (/home/alexey/shaka/popmenu/node_modules/ramda/src/internal/_xwrap.js:17:17)
    at _arrayReduce (/home/alexey/shaka/popmenu/node_modules/ramda/src/internal/_reduce.js:18:34)
    at _reduce (/home/alexey/shaka/popmenu/node_modules/ramda/src/internal/_reduce.js:60:12)
    at map (/home/alexey/shaka/popmenu/node_modules/ramda/src/map.js:77:14)             
    at /home/alexey/shaka/popmenu/node_modules/ramda/src/internal/_dispatchable.js:50:15
    at f2 (/home/alexey/shaka/popmenu/node_modules/ramda/src/internal/_curry2.js:34:14)                      
    at createPkgGraph (/home/alexey/shaka/popmenu/node_modules/@pnpm/workspace.pkgs-graph/lib/index.js:18:37)                  
    at ConfigurationChief.setWorkspaces (file:///home/alexey/shaka/popmenu/node_modules/knip/dist/ConfigurationChief.js:177:32)
    at async ConfigurationChief.init (file:///home/alexey/shaka/popmenu/node_modules/knip/dist/ConfigurationChief.js:106:9)
    at async main (file:///home/alexey/shaka/popmenu/node_modules/knip/dist/index.js:28:5)
    at async run (file:///home/alexey/shaka/popmenu/node_modules/knip/dist/cli.js:23:73)
                
Node.js v18.17.0                                                                         

This happens both without a config file and with knip.js containing

module.exports = {
  entry: [
    'app/javascript/packs/*.{js,jsx}',
    ...
  ],
};

which is my planned configuration.

Unfortunately, I can't share the project, but can test any attempted fix.

[webpro]

Knip should definitely handle this error better and more user-friendly. But this could indicate that there's a package.json in the monorepo with a missing name or version, or perhaps an incorrect specifier to an internal workspace in one of the (dev) dependencies.

[alexeyr-ci]

or perhaps an incorrect specifier to an internal workspace in one of the (dev) dependencies.

Looks like it: in node_modules/graphql-ws/package.json, there is

  "workspaces": [
    "website"
  ],

Anything I can do to work around that?

[webpro]

Sorry, I was talking about only the package.json file(s) internal to the project. Not in node_modules (Knip does not look there during the process of "Reading workspace configuration(s)").

[alexeyr-ci]

I misunderstood "in one of the (dev) dependencies" then. We don't use workspaces.

--debug gives just this before the error:

alexey@DESKTOP-41AVESA:~/shaka/popmenu$ knip --debug
[*] Unresolved configuration (from CLI arguments)
{
  cwd: '/home/alexey/shaka/popmenu',
  tsConfigFile: undefined,
  gitignore: true,
  isProduction: false,
  isStrict: false,
  isShowProgress: false,
  isIncludeEntryExports: false,
  isIsolateWorkspaces: false,
  isFix: false,
  fixTypes: []
}
/home/alexey/shaka/popmenu/node_modules/@pnpm/workspace.pkgs-graph/lib/index.js:32
            const isWorkspaceSpec = rawSpec.startsWith('workspace:');
                                            ^
... stack trace as before

Can file: or npm: in dependencies be a problem?

[alexeyr-ci]

Is it expected that it doesn't say "Reading workspace configuration(s)" with --debug?

[webpro]

Can file: or npm: in dependencies be a problem?

Nah, that's not the issue, there's something off in package.json in dependencies (or optionalDependencies or devDependencies). Maybe an invalid value or character?

It's happening here, maybe it's a bug in this dependency (not expecting you to read or debug this btw):

https://github.com/pnpm/pnpm/blob/ecda06d7b5c77131f5c4dfd4912bae9937bf2d93/workspace/pkgs-graph/src/index.ts#L50-L59

Is it expected that it doesn't say "Reading workspace configuration(s)" with --debug?

Yes, the default mode, --debug and --no-progress are all different in this regard.

[alexeyr-ci]

Ok, I've found the issue. We use the officially recommended workaround for "comments" in package.json: the "//" key. If I remove that, knip runs successfully. I guess the bug should be reported there?

[webpro]

Ah, gotcha. Yeah, that would be an issue for the @pnpm/workspace.pkgs-graph package in the pnpm repo. Thanks! Going to close this one.

[alexeyr-ci]

pnpm/pnpm#7513

[alexeyr-ci]

It turns out the problem is only an array in "//", so at worst I can keep the comments in a single string.

[alexeyr-ci]

And thank you for quick responses, by the way!

[alexeyr-ci]

Their response is that this key isn't supported in pnpm (or npm), so it isn't a bug on their side. I don't know if you can easily work around this or add an option to use corresponding yarn packages. But as I said, I have a workaround, so will try to actually use Knip next week.

[webpro]

I'm happy to look into it, but first I'd need a reproducible case.

[alexeyr-ci]

https://github.com/alexeyr-ci/knip-issue-reproduction. Simply running knip fails with this error, removing https://github.com/alexeyr-ci/knip-issue-reproduction/blob/d25baaa6d41eb6c6cb69395433cb5ac5eaf8b871/package.json#L14-L16 lets it run correctly.

[webpro]

Their response is that it this key supported in pnpm (or npm), so it isn't a bug on their side.

Sounds to me like Zoltan says npm and pnpm do not support this:

npm fails too if you put "//" into the dependencies. This is not supported.

Even if Yarn supports it, there's still not much I can do here, as indeed, I'm using @pnpm/workspace.pkgs-graph and if Knip would catch the error there's still no pkg graph.

Guess the only solution would be to re-implement this package, but I'm not sure if it's worth the hassle. What version of Yarn are you using? Since support for Yarn v1 is not a priority.

[alexeyr-ci]

Oops, sorry, you are right, I just missed a "not" while writing (plus another typo).

I think re-implementing definitely isn't worth it, and yes, it's Yarn 1.

But a simple workaround I thought of: either in getAvailableWorkspaceManifests https://github.com/webpro/knip/blob/9ba601a866a60b1cc80280637c34f910af069596/packages/knip/src/ConfigurationChief.ts#L321 or in getAvailableWorkspacePkgNames, do something like

['dependencies', 'devDependencies', 'resolutions'].forEach(key => {
  if (key in manifest) {
    delete manifest[key]['//'];
  }
});

before passing it to createPkgGraph. resolutions wasn't a problem for me, but just in case it becomes important for future versions.

So far as I can see it should be safe and not cause any other problems; this can never be a real dependency, after all. But maybe I'm overlooking something?

[alexeyr-ci]

Also optionalDependencies and peerDependencies (didn't think of them initally because they aren't used in my project). I see yarn also has bundledDependencies https://classic.yarnpkg.com/lang/en/docs/package-json/#toc-bundleddependencies, but I don't think they matter in this case. Don't know about peerDependenciesMeta, but guess they could be covered as well... but at that point, may be simpler to just iterate over all entries.