Improve knip reports by using a single config with workspaces #28491

Pike · 2025-01-15T12:27:36Z

Also move the dependency from the workspace into repo-tools.

The run with a configured workspace is slower, so I had to bump the timeout.

Hey, I just made a Pull Request!

✔️ Checklist

A changeset describing the change and affected packages. (more info)
Added or updated documentation
Tests for new functionality and regression tests for bug fixes
Screenshots attached (for UI changes)
All your commits have a Signed-off-by line in the message. (more info)

backstage-goalie · 2025-01-15T12:28:00Z

Changed Packages

Package Name	Package Path	Changeset Bump	Current Version
@backstage/repo-tools	packages/repo-tools	patch	`v0.12.1`

Pike · 2025-01-15T15:34:34Z

This only includes a changeset for repo-tools, as I'm only changing the knip-report files in the other packages. Given that those are not exposed on npm, I don't rev those packages.

Actually addressing the unused deps would be in follow-up PRs.

CC @freben, we quickly talked about the knip false-positives on discord. Not quite yet at the point where there are none, but much closer.

Pike · 2025-01-15T15:51:22Z

PS: the following packages don't have reports at all yet, not sure if it's good to add them in this PR:

packages/backend-common
packages/frontend-defaults
packages/yarn-plugin
packages/canon
packages/frontend-internal
packages/scaffolder-internal
packages/opaque-internal
plugins/auth-backend-module-bitbucket-provider
plugins/catalog-unprocessed-entities-common
plugins/proxy-node
plugins/app
plugins/user-settings-common
plugins/auth-backend-module-bitbucket-server-provider
plugins/notifications-backend-module-email
plugins/catalog-backend-module-gitlab-org
plugins/auth-backend-module-azure-easyauth-provider
plugins/catalog-backend-module-logs
plugins/techdocs-common
plugins/scaffolder-backend-module-notifications
plugins/auth-backend-module-auth0-provider
plugins/auth-react

freben · 2025-01-15T17:15:13Z

packages/repo-tools/src/commands/knip-reports/knip-extractor.ts

+        entry: [
+          'dev/index.{ts,tsx}',
+          'src/index.{ts,tsx}',
+          'src/alpha.{ts,tsx}',


Would be nice to generate this based on the actual package.json files. Some packages have MANY entrypoints (notably backend-defaults).

Regarding extensions, we're adding more in #19855

entrypoints work, I just cross-checked backend-defaults.

This fixes alpha, we only have two packages (I think), which don't expose an existing src/alpha.ts:

grep -L ./alpha `ls p*/*/src/alpha.ts|sed -e s'|src/alpha.ts|package.json|'` packages/cli/package.json plugins/scaffolder-backend-module-sentry/package.json

The scaffolder one is empty, not sure about the cli one?

This won't cover the dev directories for the dev server, though.

I tested dropping src/index.{ts,tsx}, but that triggered more false positives. I didn't dig in deep, but repo-tools is one of them. It only exposes the dist cjs, which one may not have. This might need some more dedicated research.

Re extensions, is that the right PR? That's 2023?

Ahh no I meant #28308

What I meant with the entrypoints is, does it understand that for example this file is an entirely separate entrypoint too, and that any dependencies it uses need to be taken into account as well, in addition to the root index?

Yes, excerpt from running knip --debug:

[packages/backend-defaults] Finding package.json entry paths { patterns: [ 'packages/backend-defaults/package.json', 'packages/backend-defaults/src/entrypoints/userInfo/index.ts', 'packages/backend-defaults/src/entrypoints/urlReader/index.ts', 'packages/backend-defaults/src/entrypoints/scheduler/index.ts', 'packages/backend-defaults/src/entrypoints/rootLogger/index.ts', 'packages/backend-defaults/src/entrypoints/rootLifecycle/index.ts', 'packages/backend-defaults/src/entrypoints/rootHttpRouter/index.ts', 'packages/backend-defaults/src/entrypoints/rootHealth/index.ts', 'packages/backend-defaults/src/entrypoints/rootConfig/index.ts', 'packages/backend-defaults/src/entrypoints/permissions/index.ts', 'packages/backend-defaults/src/entrypoints/permissionsRegistry/index.ts', 'packages/backend-defaults/src/entrypoints/logger/index.ts', 'packages/backend-defaults/src/entrypoints/lifecycle/index.ts', 'packages/backend-defaults/src/entrypoints/httpRouter/index.ts', 'packages/backend-defaults/src/entrypoints/httpAuth/index.ts', 'packages/backend-defaults/src/entrypoints/discovery/index.ts', 'packages/backend-defaults/src/entrypoints/database/index.ts', 'packages/backend-defaults/src/entrypoints/cache/index.ts', 'packages/backend-defaults/src/entrypoints/auth/index.ts', 'packages/backend-defaults/src/index.ts' ], ... paths: [ '/Users/54717/src/backstage/backstage/packages/backend-defaults/package.json', '/Users/54717/src/backstage/backstage/packages/backend-defaults/src/entrypoints/userInfo/index.ts', '/Users/54717/src/backstage/backstage/packages/backend-defaults/src/entrypoints/urlReader/index.ts', '/Users/54717/src/backstage/backstage/packages/backend-defaults/src/entrypoints/scheduler/index.ts', '/Users/54717/src/backstage/backstage/packages/backend-defaults/src/entrypoints/rootLogger/index.ts', '/Users/54717/src/backstage/backstage/packages/backend-defaults/src/entrypoints/rootLifecycle/index.ts', '/Users/54717/src/backstage/backstage/packages/backend-defaults/src/entrypoints/rootHttpRouter/index.ts', '/Users/54717/src/backstage/backstage/packages/backend-defaults/src/entrypoints/rootHealth/index.ts', '/Users/54717/src/backstage/backstage/packages/backend-defaults/src/entrypoints/rootConfig/index.ts', '/Users/54717/src/backstage/backstage/packages/backend-defaults/src/entrypoints/permissions/index.ts', '/Users/54717/src/backstage/backstage/packages/backend-defaults/src/entrypoints/permissionsRegistry/index.ts', '/Users/54717/src/backstage/backstage/packages/backend-defaults/src/entrypoints/logger/index.ts', '/Users/54717/src/backstage/backstage/packages/backend-defaults/src/entrypoints/lifecycle/index.ts', '/Users/54717/src/backstage/backstage/packages/backend-defaults/src/entrypoints/httpRouter/index.ts', '/Users/54717/src/backstage/backstage/packages/backend-defaults/src/entrypoints/httpAuth/index.ts', '/Users/54717/src/backstage/backstage/packages/backend-defaults/src/entrypoints/discovery/index.ts', '/Users/54717/src/backstage/backstage/packages/backend-defaults/src/entrypoints/database/index.ts', '/Users/54717/src/backstage/backstage/packages/backend-defaults/src/entrypoints/cache/index.ts', '/Users/54717/src/backstage/backstage/packages/backend-defaults/src/entrypoints/auth/index.ts', '/Users/54717/src/backstage/backstage/packages/backend-defaults/src/index.ts' ] }

I rebased this ontop of #28308 now that that's merged, and I needed some adjustment for the tests.

Otherwise, this works like expected, I think.

freben · 2025-01-15T17:17:11Z

packages/frontend-test-utils/knip-report.md

+
+| Name               | Location                          | Severity |
+| :----------------- | :-------------------------------- | :------- |
+| @internal/frontend | src/app/createExtensionTester.tsx | error    |


Would be neat to not list the internal ones. They are marked as such in package.json, which is a better way of detect them than by name.

Yeah, I think this is something we need to discuss. This might be a topic to bring up with the knip folks upstream.

I wonder if a mono-repo and "private": true, in package.json would be enough of a signal. Though, looking at https://github.com/search?q=repo%3Awebpro-nl%2Fknip+private+path%3A%2F%5Epackages%5C%2Fknip%5C%2Ffixtures%5C%2F%2F&type=code it seems that that signal is already used in their test fixtures.

The

"backstage": { "role": "web-library", "inline": true },

part would only be available to plugins, which need to be upstream. And I don't see a way to extend ignored dependencies from those. Another thing to check.

Should I just add something name-based for now?

Yeah it's the Backstage specific "online" concept I meant indeed.

Maybe let's leave it as a future improvement. It's still a lot better than before either way.

I went with a regexp on the name for now.

Also move the dependency from the workspace into repo-tools. The run with a configured workspace is slower, so I had to bump the timeout. Signed-off-by: Axel Hecht <axel@pike.org>

Also remove a stray one Signed-off-by: Axel Hecht <axel@pike.org>

Let's figure out later what triggers those Signed-off-by: Axel Hecht <axel@pike.org>

Signed-off-by: Axel Hecht <axel@pike.org>

Using just the naming convention for now. Signed-off-by: Axel Hecht <axel@pike.org>

Canon uses a different setup for storybook Signed-off-by: Axel Hecht <axel@pike.org>

Signed-off-by: Axel Hecht <axel@pike.org>

Pike · 2025-01-16T14:45:24Z

PS: the following packages don't have reports at all yet, not sure if it's good to add them in this PR:

I back-filled the missing reports now, too. Only canon is not done, because that has a storybook setup that differs from the rest, so I'd recommend that to be done in a follow-up.
I also enabled the techdocs-cli-embed one again, I couldn't reproduce the problem that was suggested in the comment to disable it.

@freben , this should be ready for another look.

Pike force-pushed the knip-workspace-config branch from 1862ff0 to 845e2b7 Compare January 15, 2025 15:28

Pike marked this pull request as ready for review January 15, 2025 15:29

Pike requested review from a team, awanlin, pjungermann and backstage-service as code owners January 15, 2025 15:29

Pike requested review from benjdlambert and vinzscam January 15, 2025 15:29

freben reviewed Jan 15, 2025

View reviewed changes

Pike force-pushed the knip-workspace-config branch from 7578146 to 8e0de41 Compare January 16, 2025 10:53

Pike added 8 commits January 16, 2025 14:36

Improve knip reports by using a single config with workspaces

3e2e0a1

Also move the dependency from the workspace into repo-tools. The run with a configured workspace is slower, so I had to bump the timeout. Signed-off-by: Axel Hecht <axel@pike.org>

Update existing knip-report files

1c8245f

Also remove a stray one Signed-off-by: Axel Hecht <axel@pike.org>

Ignore optional peerDependencies

fee272c

Let's figure out later what triggers those Signed-off-by: Axel Hecht <axel@pike.org>

Move ignores into workspace to fix templates

645e942

Signed-off-by: Axel Hecht <axel@pike.org>

Slim down entries, relying on package.json exports

4d55e45

Signed-off-by: Axel Hecht <axel@pike.org>

Ignore internal non-published packages

a369c27

Using just the naming convention for now. Signed-off-by: Axel Hecht <axel@pike.org>

Add missing reports, only skip canon

efcbe08

Canon uses a different setup for storybook Signed-off-by: Axel Hecht <axel@pike.org>

Fix for cli packaging tests

7e5fa38

Signed-off-by: Axel Hecht <axel@pike.org>

Pike force-pushed the knip-workspace-config branch from 8e0de41 to 7e5fa38 Compare January 16, 2025 14:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve knip reports by using a single config with workspaces #28491

Improve knip reports by using a single config with workspaces #28491

Pike commented Jan 15, 2025

backstage-goalie bot commented Jan 15, 2025 •

edited

Loading

Pike commented Jan 15, 2025

Pike commented Jan 15, 2025

freben Jan 15, 2025

Pike Jan 15, 2025

freben Jan 15, 2025

freben Jan 15, 2025

Pike Jan 16, 2025

Pike Jan 16, 2025

freben Jan 15, 2025

Pike Jan 15, 2025

freben Jan 15, 2025

Pike Jan 16, 2025

Pike commented Jan 16, 2025

Improve knip reports by using a single config with workspaces #28491

Are you sure you want to change the base?

Improve knip reports by using a single config with workspaces #28491

Conversation

Pike commented Jan 15, 2025

Hey, I just made a Pull Request!

✔️ Checklist

backstage-goalie bot commented Jan 15, 2025 • edited Loading

Changed Packages

Pike commented Jan 15, 2025

Pike commented Jan 15, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Pike commented Jan 16, 2025

backstage-goalie bot commented Jan 15, 2025 •

edited

Loading