⚠️ No Changeset found

Latest commit: f233b2f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

The workspaces config I used for examples and templates was incorrect (examples instead of examples/*). Actually enabling them creates a bunch of extra work like needing to install all of their dependencies so I just removed it and will revisit later maybe

Linked from webpro-nl/knip#735, thanks for trying Knip! I saw the config and thought I might be able to help out a little.

From running Knip in this repo, I was able to fix two bugs, thanks! Re. babel overrides and old husky.

Knip automatically picks up package.json#workspaces, but looks like we can use scripts/*.js for the root and then the rest seems to work out fine.

Now we can simplify the config a bit:

{
  "$schema": "https://unpkg.com/knip@5/schema.json",
  "workspaces": {
    ".": {
      "entry": ["scripts/*.js"],
      "project": ["scripts/*.js"]
    }
  },
  "ignore": [
    // used for `#is-development` conditional import
    "packages/**/{true,false}.ts",
    // file acts as a type test
    "packages/xstate-svelte/test/interpreterAsReadable.svelte"
  ],
  "ignoreBinaries": ["svelte-check", "docs:build"],
  "ignoreDependencies": [
    "@xstate-repo/jest-utils",
    "@xstate-repo/jest-utils/setup",
    "synckit",
    // package.json#exports aren't added as entry points, because `dist/` is .gitignored
    "react",
    "xstate",
    "@types/ws"
  ]
}

There's remaining output:

❯ knip
Unused devDependencies (2)
@tsconfig/svelte  packages/xstate-svelte/package.json
svelte-jester     packages/xstate-svelte/package.json
Unlisted dependencies (1)
svelte-jester  jest.config.js

• @tsconfig/svelte is confusing because of packages/xstate-svelte/test/package.json - maybe just add to ignoreDependencies
• I guess svelte-jester could be moved to the root package.json

You could try now using yarn add -D https://pkg.pr.new/knip@bd16c98 or wait a little for the next GA.

AMA :)

@tsconfig/svelte is confusing because of packages/xstate-svelte/test/package.json - maybe just add to ignoreDependencies

I assume this might be some issue with pkg.json lookup. Such "package.json scopes" to add type are... weird but I've seen them used them in the wild a couple of times already. Maybe it would be worth ignoring them in Knip? This is a documented pattern in the node.js docs.

@webpro Thanks so much for taking a look! Glad you were able to fix two bugs while helping us clean up the config. And also glad to learn about pkg.pr.new, that's quite a useful tool.

I was thinking about posting on twitter to ask if I knew any knip experts that wanted to take a look at the config but opening an issue got us the expert. The power of open source ❤️

I believe this is good to go now 😁

@tsconfig/svelte is confusing because of packages/xstate-svelte/test/package.json - maybe just add to ignoreDependencies

I assume this might be some issue with pkg.json lookup. Such "package.json scopes" to add type are... weird but I've seen them used them in the wild a couple of times already. Maybe it would be worth ignoring them in Knip? This is a documented pattern in the node.js docs.

Sorry, not sure exactly what you are referring to here. Knip follows the root package.json#workspaces to find workspaces (extra workspaces can be configured, though). If we ignore packages/xstate-svelte/test/package.json in this regard, then the easiest path is to configure test/tsconfig.json as the path of the TypeScript config for this workspace:

{
  "workspaces": {
    "packages/xstate-svelte": {
      "typescript": "test/tsconfig.json",
      // shorthand for "typescript": { "config": ["test/tsconfig.json"] }
    }
  }
}

Since [email protected] was released this morning, I've replaced pkg.pr.new with ^5.27.0. Running it results in no errors so we should be good to go! 🤞

Thanks @with-heart! ➕❤️

You could also use the config from my latest comment to get rid of of the ignored @tsconfig/svelte issue, but it's minor.

I guess I just don't understand why this has to be declared as an extra workspace in the first place - couldn't it be picked up automatically? It's fine as a solution right now - I just wonder how Knip's magic sauce is made and if there is any room for improvements on that front

I guess I just don't understand why this has to be declared as an extra workspace in the first place - couldn't it be picked up automatically?

There's no magic here, Knip just uses package.json#workspaces - which does not include packages/xstate-svelte/test.

Right, because it's not a workspace. But including it in Knip's workspaces it feels like we dilute the meaning of a workspace. Multiple tsconfig.jsons (and package.jsons for that matter) are fair game within a single workspace. So I just wonder if they couldn't be discovered automatically? It would require less work for the user.

Right, because it's not a workspace. But including it in Knip's workspaces it feels like we dilute the meaning of a workspace.

We're not including it as a workspace, it's only Knip's TypeScript plugin that pulls in test/tsconfig.json instead of ./tsconfig.json so it can extract dependencies from that file. There's nothing more to it, it's not like we're defining ./test as a TS project or a (npm/Yarn) workspace.

Multiple tsconfig.jsons (and package.jsons for that matter) are fair game within a single workspace. So I just wonder if they couldn't be discovered automatically? It would require less work for the user.

Unfortunately we're simply not there yet.

Is this one good to go on (pending merge conflicts)? @Andarist @with-heart

Yeap we're good to go!