Skip to content

Load plugin configs from workspace cwd#1763

Closed
jakeleventhal wants to merge 1 commit into
webpro-nl:mainfrom
jakeleventhal:codex/fix-graphql-codegen-load-fallback
Closed

Load plugin configs from workspace cwd#1763
jakeleventhal wants to merge 1 commit into
webpro-nl:mainfrom
jakeleventhal:codex/fix-graphql-codegen-load-fallback

Conversation

@jakeleventhal

@jakeleventhal jakeleventhal commented May 22, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Load JavaScript/TypeScript plugin config files with the workspace/package cwd, while restoring the original process cwd afterward.
  • Keep the cwd switch scoped to plugin config loading instead of changing the generic loader API/behavior for every load() caller.
  • Add a GraphQL Codegen regression fixture whose --config ./src/codegen.ts file reads package-relative config and only resolves when evaluated from the package cwd.

Root Cause

graphql-codegen --config ./src/codegen.ts is discovered from the package script, so the config should be evaluated the same way that package script runs: from the workspace/package cwd. When Knip loads the config from the repo-root cwd, relative paths inside the config resolve incorrectly and plugin dependencies referenced by the loaded config can be reported as unused.

Validation

  • node --test packages/knip/test/plugins/graphql-codegen.test.ts
  • pnpm exec oxfmt --check packages/knip/src/util/loader.ts packages/knip/src/util/plugin.ts packages/knip/test/plugins/graphql-codegen.test.ts packages/knip/fixtures/plugins/graphql-codegen-script-config-cwd-context/**/*.ts packages/knip/fixtures/plugins/graphql-codegen-script-config-cwd-context/**/*.json
  • pnpm run --dir packages/knip lint
  • pnpm run --dir packages/knip build
  • No-cache riptech validation with the GraphQL Codegen ignore removed and the original relative dotenv path left in place: node /Users/jakeleventhal/Documents/Codex/2026-05-22/update-my-pr-based-on-this/knip/packages/knip/src/cli.ts --directory /Users/jakeleventhal/Developer/rip-technologies --no-progress --workspace @artelo/integrations

@jakeleventhal jakeleventhal changed the title [codex] Fix GraphQL Codegen dependencies when config load fails Fix GraphQL Codegen dependencies when config load fails May 22, 2026
@jakeleventhal jakeleventhal marked this pull request as ready for review May 22, 2026 13:09
@jakeleventhal

Copy link
Copy Markdown
Contributor Author

Not sure if you have a better approach here

@pkg-pr-new

pkg-pr-new Bot commented May 22, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/knip@1763
npm i https://pkg.pr.new/@knip/language-server@1763
npm i https://pkg.pr.new/@knip/mcp@1763

commit: 079846f

@webpro

webpro commented May 22, 2026

Copy link
Copy Markdown
Member

AST resolver is fine (better), but ideally it's either/or (not both resolveConfig and resolveFromAST).

Btw documentation for users at https://knip.dev/reference/known-issues#exceptions-from-config-files

@jakeleventhal jakeleventhal force-pushed the codex/fix-graphql-codegen-load-fallback branch from 2d007b3 to eb02261 Compare May 22, 2026 13:17
@jakeleventhal jakeleventhal changed the title Fix GraphQL Codegen dependencies when config load fails [codex] Load plugin configs from workspace cwd May 22, 2026
@jakeleventhal jakeleventhal changed the title [codex] Load plugin configs from workspace cwd Load plugin configs from workspace cwd May 22, 2026
@jakeleventhal

Copy link
Copy Markdown
Contributor Author

@webpro

Thanks. I reworked this to avoid combining resolveConfig and resolveFromAST.

Looking at the repro again, the issue is not that the config throws and Knip should recover with AST. It’s that Knip evaluates the --config ./src/codegen.ts file from the repo-root cwd, while the package script runs from the workspace cwd. Relative paths inside the config then resolve differently.

The updated PR keeps the existing resolveConfig path and changes plugin config loading to temporarily use the workspace cwd.

@webpro

webpro commented May 22, 2026

Copy link
Copy Markdown
Member

While I understand it should just work and it's on Knip, yet still: why not follow good practice and use resolve or join for absolute path? Not sure if we should add chdir() for each load() invocation.

@jakeleventhal jakeleventhal changed the title Load plugin configs from workspace cwd Add GraphQL Codegen script config absolute path fixture May 22, 2026
@jakeleventhal jakeleventhal force-pushed the codex/fix-graphql-codegen-load-fallback branch from 6f3bfc8 to 079846f Compare May 22, 2026 20:28
@jakeleventhal jakeleventhal changed the title Add GraphQL Codegen script config absolute path fixture Load plugin configs from workspace cwd May 22, 2026
@jakeleventhal

Copy link
Copy Markdown
Contributor Author

@webpro check again

@webpro

webpro commented May 27, 2026

Copy link
Copy Markdown
Member

Thanks! Much better, performance is no longer an argument against this. However, I'm still on the fence with this one, because with this type of change there's basically no going back once merged and it blocks potential future parallel work.

@jakeleventhal

Copy link
Copy Markdown
Contributor Author

@webpro what's the path forward here?

@webpro

webpro commented Jun 2, 2026

Copy link
Copy Markdown
Member

Let's close it and not shoot ourselves in the foot. I'll close it by updating docs soon.

@jakeleventhal

Copy link
Copy Markdown
Contributor Author

@webpro then how can I solve cases like this: https://github.com/jakeleventhal/knip-1763-repro

@webpro

webpro commented Jun 2, 2026

Copy link
Copy Markdown
Member

That link 404s. Probably I would answer:

follow good practice and use resolve or join for absolute path

@jakeleventhal

Copy link
Copy Markdown
Contributor Author

err, sorry can you check again please @webpro

@webpro webpro closed this in fcd444b Jun 3, 2026
@webpro

webpro commented Jun 5, 2026

Copy link
Copy Markdown
Member

🚀 This issue has been resolved in v6.16.0. See Release 6.16.0 for release notes.

Using Knip in a commercial project? Please consider becoming a sponsor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants