Ignore "//" key in (dev)depedencies about knip HOT 17 CLOSED

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.

from knip.

And thank you for quick responses, by the way!

from knip.

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.

from knip.

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?

from knip.

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)").

from knip.

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?

from knip.

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

from knip.

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.

from knip.

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?

from knip.

pnpm/pnpm#7513

from knip.

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

from knip.

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.

from knip.

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

from knip.

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.

from knip.

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.

from knip.

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?

from knip.

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.

from knip.