Skip to content

fix: honor explicit entry export analysis - #2012

Merged
webpro merged 4 commits into
webpro-nl:mainfrom
gioboa:fix/2011
Sep 7, 2026
Merged

webpro merged 4 commits into
webpro-nl:mainfrom
gioboa:fix/2011

Conversation

@gioboa

@gioboa gioboa commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Closes #2011

Explicit workspace entries now override script-derived export exemptions and avoid incorrect redundancy hints.

@pkg-pr-new

pkg-pr-new Bot commented Sep 4, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

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

commit: df069d1

@usetheodev

Copy link
Copy Markdown

Reporter of #2011 here — I built this branch and ran it against the fixture from that issue, plus a control. It fixes the case I filed.

Verified

Fixture from #2011 (root scripts.dev naming a file inside a workspace, includeEntryExports: true, entry: ["src/main.ts"] in the workspace):

build result
main @ c6497a1 Configuration hints (1) src/main.ts … Remove redundant entry pattern — exit 0, DEAD not reported
this branch @ 2e88de4 Unused exports (1) DEAD packages/app/src/main.ts:1:14 — exit 1

The misleading hint is gone too, which was half of what made this hard to notice.

The control matters: main was rebuilt from source and still reproduces, so the fix is this PR and not something between 6.32.2 (where I originally measured) and 6.34.0.

It does real work outside the fixture

Run against the repository where I found this — a pnpm monorepo whose workspace entry globs are the test files:

main @ c6497a1     exit 0, clean
+ this PR          exit 1
                   Unused exports (1)
                   omittingAGateMustNotCompile  packages/agent/src/hooks/fail-safe-defaults.test.ts:43:17

A true positive: a compile-time assertion helper, exported only so eslint would not flag an unused local. Its own docblock says "the dead-export scan lists it" — written by someone who assumed it was covered. It had never been reported, because test files are that workspace's declared entries. Exempting it locally with tags: ["-knipignore"] + /** @knipignore */ silences it cleanly, verified on this branch.

So the change surfaces genuinely invisible exports, not just fixture ones.

One case still exempt — asking rather than asserting

When the file is an entry only through the root script, with no explicit entry for it in the workspace:

// knip.jsonc
"workspaces": { "packages/app": { "entry": ["src/**/*.test.ts"], "project": ["src/**/*.ts"] } }
// root package.json
"scripts": { "dev": "node packages/app/src/main.ts" }

DEAD in packages/app/src/main.ts is still not reported on this branch (exit 0).

That may be exactly the intended semantics — an explicit declaration is a signal, and without one a script-derived entry is a program entry point whose exports are arguably its surface. #2011 asked the same question and I have no stake in the answer; I only want the boundary written down somewhere, since the option is named includeEntryExports and that case is an entry whose exports are not included.

If it is intended, the doc line is enough and the remaining fix for consumers is "declare the entry" — which this PR now makes work.

Reproduce

git fetch origin pull/2012/head:pr2012 && git switch pr2012
cd packages/knip && npx tsc
node packages/knip/bin/knip.js --no-progress   # from the fixture above

Thanks for turning this around so fast.

@gioboa gioboa changed the title Honor explicit entry export analysis fix: honor explicit entry export analysis Sep 4, 2026
usetheodev pushed a commit to usetheoai-lab/TheoCode that referenced this pull request Sep 4, 2026
webpro-nl/knip#2012 landed as a proposal against the issue this repository
filed. Built from source and measured rather than read.

It fixes the case reported: with an explicit workspace entry, the dead export
is reported and the misleading "Remove redundant entry pattern" hint is gone.
The control matters — knip main rebuilt from the same source still reproduces,
so the fix is that PR and not a version change between 6.32.2 and 6.34.0.

Against this tree it is not a no-op. The run goes from clean to one finding:
omittingAGateMustNotCompile in fail-safe-defaults.test.ts, reachable now
because the test globs ARE this workspace's declared entries. A true positive
and a deliberate survivor — a compile-time assertion helper exported only
because eslint flags an unused local, whose own docblock already said the scan
would list it. The remedy is local and was verified on that branch:
tags: ["-knipignore"] plus /** @knipignore */ beside the reason.

The four main/entry files stay exempt even with the PR, because none is
declared in any entry list — only the root scripts make them entries. Adding
them is left undone on purpose: the PR is unmerged, and a config written
against unreleased behaviour is a gate nobody can verify today.

Verification posted on the PR, including the one case that remains exempt,
framed as a question about intended semantics rather than a demand.

Refs #71
usetheodev pushed a commit to usetheoai-lab/TheoCode that referenced this pull request Sep 6, 2026
Bumped and then measured rather than bumped and assumed. The defect #71 tracks
survives the newer release, with both arms run on it:

    dead export in an ENTRY file      -> not reported   (the gap)
    the same export in a NON-entry    -> reported       (positive control)

The control is what makes the first line evidence rather than silence: it proves
the instrument still speaks on this version, so the zero is about `includeEntryExports`
and not about the run.

`#71` therefore stays open on `webpro-nl/knip#2012`, which is still unmerged and
not in 6.34.0. What changes is the report's footing — the measurement now cites
the latest published version instead of the one the issue was written against,
so nobody has to wonder whether it was already fixed.

Lint and the full suite green on the new version.

@webpro webpro left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for this. The entry precedence looks right on uncached runs, but configuration changes can leave cached results stale. I'd like to see that fixed before merging.

Comment thread packages/knip/src/ProjectPrincipal.ts
@webpro

webpro commented Sep 7, 2026

Copy link
Copy Markdown
Member

Thanks!

Pushed a nit (test failed against the old implementation).

@webpro
webpro merged commit a092e40 into webpro-nl:main Sep 7, 2026
30 checks passed

@gioboa gioboa left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks @webpro

@gioboa
gioboa deleted the fix/2011 branch September 7, 2026 12:43
@webpro

webpro commented Sep 8, 2026

Copy link
Copy Markdown
Member

🚀 This pull request is included in v6.35.0. See Release 6.35.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.

includeEntryExports has no effect when a package.json script makes the file an entry (monorepo)

3 participants