Comments (6)
Closing this with v5.17.0, feel free to discuss further or open a new issue.
from knip.
This is an interesting case, could be a challenge to resolve it.
Assuming the intention of allFixtures.order1
is allFixtures.orderFixtures.order1
.
And I'm afraid this is a "bridge too far" currently for Knip, the chain breaks here:
import * as orderFixtures from './orderFixtures';
export const allFixtures = { orderFixtures };
Interestingly, trying "Find all references" in my IDE on the order1
export does not lead me to src/App.jsx
. Also TS itself has trouble here, this surprises me. the other way around is fine, from usage to implementation.
The issue here is that it's a property on a new export. It's not an explicit or even implicit re-export:
// explicit
export * from './orderFixtures';
// implicit
import * as orderFixtures from './orderFixtures';
export { orderFixtures };
This kind of works, would come close:
import * as orderFixtures from './orderFixtures';
export { orderFixtures as allFixtures };
Obviously not what you want, but just to show what Knip does support technically. In this last case all exports on the orderFixtures
namespace are now considered used, which is something that could/should be improved on. In this scenario I also see that TS "Find all references" leads me to src/App.jsx
so this scenario is something we could try and fix in Knip.
Your use case is totally valid JS/TS, it's just not something I see easily fixed in Knip.
If you're interested in working on improving this last case I'm happy to elaborate on the whereabouts of related things :)
from knip.
Couldn't resist and went ahead on this myself.
This use case should be fixed in v5.14.0 and should properly handle exports individually (before, it assumed all members of the namespace as referenced):
import * as orderFixtures from './orderFixtures';
export { orderFixtures as allFixtures };
During the refactor I went ahead and tried whether supporting the original use case is feasible. I've added support in 5.15.0-keyedreexport.0
. Not 100% sure yet I'll add it, but let's see where this goes. Any chance you could give the tagged keyedreexport
version a shot on your workload? You can install using e.g.
npm install -D knip@keyedreexport
Happy to hear your feedback, @glemiron!
from knip.
From TS compiler internals Discord:
I've just come across a case where I think
findReferences
might have a bug:
- ✅ https://codesandbox.io/p/devbox/github/webpro/knip/main/packages/knip/fixtures/re-exports-aliased-ns?file=%2F1-first.ts%3A1%2C16 - find references of
first
leads toindex.ts
- 🔴 https://codesandbox.io/p/devbox/github/webpro/knip/feat/extend-ns-reexports/packages/knip/fixtures/re-exports-keyed-ns?file=%2F1-first.ts%3A1%2C14 - find references of
first
does not lead toindex.ts
- the difference is how NS is "re-exported" from 4-collect.ts
from knip.
Unfortunately it seems that the fix doesn't work. In order to eliminate set up problems I've created a demo repository here.
I also found the second use case, I think it's related and you can also check out it in the repository.
And another observation: WebStorm is detects unused code correctly, so it seems as solvable problem.
from knip.
Thanks for the demo repo! This is super helpful. Let's break it down a little bit.
- First case:
import * as orderFixtures from './orderFixtures'
export const allFixtures = { orderFixtures: orderFixtures }; // NOT supported (yet?)
export const allFixtures = { orderFixtures }; // Supported in @keyedreexport
To this I mentioned that I was "assuming allFixtures.orderFixtures.order1
", and the last line in this example is the case that I think is fixed in the keyedreexport
release.
- Next case. This should be fixed and works as expected in main/v5.14.0 (and
keyedreexport
as well):
import * as orderFixtures from './orderFixtures';
export { orderFixtures as allFixtures };
- The case that's brought in is this one:
import * as orderFixtures from './orderFixtures'
export const allFixtures = orderFixtures
While valid, but this could also (should?) be written like so:
import * as orderFixtures from './orderFixtures'
export { orderFixtures as allFixtures }
This case worked fine in main prior to opening this issue. Or maybe there's additional code/syntax missing that we've not seen/covered yet?
- Last but not least you've introduced this case:
import { ValuesType } from 'utility-types'
import * as FEATURES from './constants'
export type Feature = ValuesType<typeof FEATURES>
This is a case that wasn't covered yet and is now added in v5.15.1.
And another observation: WebStorm is detects unused code correctly, so it seems as solvable problem.
Interesting, didn't know about this feature. Do you mean this Find unused code with coverage? That would be a bit different: Knip is a static analysis tool, and I think this feature is instrumenting code and then doing the analysis more dynamically in runtime.
That said, it's a solvable problem indeed. For now I'm going to keep it short here by saying that doing "Find all references" for each export takes too long in large projects (also see Slim down to speed up), but this can still be enabled by using --include-libs
(which was added for a different feature and thus is unclear in this context and should be renamed).
from knip.
Related Issues (20)
- 🔄 Issue links are broken HOT 4
- 🐛 False positive for `unlisted` when extending a tsconfig file from a package HOT 4
- (PostCSS plugin): Add `postcss` to `referencedDependencies` when using Tailwind CSS with PostCSS HOT 2
- 💡 Add option to ignore export names using a pattern HOT 1
- 🐛 Soft-linked dependencies are considered unused HOT 2
- 🐛 Abstracted lazy imports are not detected HOT 1
- 💡 Replace `fast-glob` by `tinyglobby` HOT 5
- 🐛 false positive in Astro when an import follows an import assertion import HOT 2
- Knip doesn't enable Vue plugin in Nuxt 🐛 HOT 6
- 🐛 Dev dependencies are reported as unused HOT 1
- 🔄 Regression on importing from subdirectory of a package in node_modules HOT 5
- 💡 Have a mode or configuration option that allows for monorepos that install deps at the root HOT 5
- 🐛 Not following `references` in TS config files HOT 13
- 💡 Follow `references` in TS config file. HOT 1
- 🐛 Webpack config not found when it's not in root folder HOT 4
- 💡 `ignoreImportsUsedInFiles: []` HOT 1
- 💡 docs should have hover links HOT 1
- 🐛 `vitest.workspace.mts` Not Dectected by Vitest Plugin HOT 2
- 🐛 Impossible to Ignore Env Var False Positives in Yarn Scripts HOT 1
- 💡 Support `jsconfig.json` for import path aliases by default HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from knip.