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

chore(tooling): install knip and remove unused files / dependencies #3106

Open
wants to merge 7 commits into
base: next
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/renovate.json5
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
},
{
"groupName": "doc-dependencies",
"matchPackageNames": ["@algolia/client-search", "ts-morph", "vitepress"]
"matchPackageNames": ["ts-morph", "vitepress"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably add vue/vue-tsc/@vueuse/core in here too. (Different PR)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will create a new PR for that 🙏🏻

}
],
"stopUpdatingLabel": "s: on hold",
Expand Down
14 changes: 14 additions & 0 deletions knip.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { type KnipConfig } from 'knip';
jeanpierrecarvalho marked this conversation as resolved.
Show resolved Hide resolved

const config: KnipConfig = {
workspaces: {
jeanpierrecarvalho marked this conversation as resolved.
Show resolved Hide resolved
'.': {
entry: ['src/**/*', 'test/**/*'],
ignore: ['docs/**/*', 'scripts/**/*', '.prettierrc.d.ts', '.github/**/*'],
ignoreDependencies: ['@actions/github', '@vueuse/core', 'vue'],
ignoreBinaries: [],
},
},
};

export default config;
12 changes: 3 additions & 9 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@
"docs:test:e2e:open": "run-p --race docs:serve \"cypress open\"",
"release": "commit-and-tag-version",
"prepublishOnly": "pnpm run clean && pnpm install && pnpm run build",
"preflight": "pnpm install && run-s generate format lint build test:update-snapshots ts-check"
"preflight": "pnpm install && run-s generate format lint build test:update-snapshots ts-check",
"knip": "knip"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this something we should run as part of preflight? Or in our CI to find these as early as possible?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preflight is already slow enough 😀

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just ran the preflight and yes, it's painful ahhahahah Do we need to do this preflight like that? Why the process is not splited using git hooks e.g and run, only if needed. We can perform some tasks only in the files in stage, using lint-staged e.g, WDYT? I can draw a proposal and explain my pov better

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think it would be good to open a separate discussion on ways to speed up preflight

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed! I will create that discussion and add my thought there 🙏🏻

},
"keywords": [
"faker",
Expand Down Expand Up @@ -94,12 +95,6 @@
],
"devDependencies": {
"@actions/github": "6.0.0",
"@algolia/client-search": "5.2.3",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this dependency is now, optional. It would be good if somebody could test that as well.

Thinking: I wonder when that change happened.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please create a separate PR that remove this dependency. That way it is easier to test for breaking changes.

"@eslint-types/deprecation": "2.0.0-1",
"@eslint-types/jsdoc": "48.2.2",
"@eslint-types/prettier": "5.1.3",
"@eslint-types/typescript-eslint": "7.5.0",
"@eslint-types/unicorn": "52.0.0",
jeanpierrecarvalho marked this conversation as resolved.
Show resolved Hide resolved
"@eslint/compat": "1.1.1",
"@eslint/js": "9.9.1",
"@stylistic/eslint-plugin": "2.7.2",
Expand All @@ -114,11 +109,11 @@
"cypress": "13.14.1",
"eslint": "9.9.1",
"eslint-config-prettier": "9.1.0",
"eslint-define-config": "2.1.0",
"eslint-plugin-jsdoc": "50.2.2",
"eslint-plugin-prettier": "5.2.1",
"eslint-plugin-unicorn": "55.0.0",
"eslint-plugin-vitest": "0.5.4",
"knip": "^5.30.1",
jeanpierrecarvalho marked this conversation as resolved.
Show resolved Hide resolved
"npm-run-all2": "6.2.2",
"prettier": "3.3.3",
"prettier-plugin-organize-imports": "4.0.0",
Expand All @@ -132,7 +127,6 @@
"typescript": "5.5.4",
"typescript-eslint": "8.3.0",
"validator": "13.12.0",
"vite": "5.4.2",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please create a separate PR that remove this dependency. That way it is easier to test for breaking changes.

"vitepress": "1.3.4",
"vitest": "2.0.5",
"vue": "3.4.38",
Expand Down
Loading