https://stackblitz.com/edit/github-yu5k5m-vxck8v?file=index.ts
Reopening issue #616 as per the closing comment.
React lazy's default implementation creates for some undesirable behavior, so many projects will wrap it using a factory pattern. Knip's lack of support for this (emitting false positives) makes it a present non-starter for many large react projects.
The following code is properly caught by Knip
const Component2 = lazy(() => import('./Bar').then(({ Bar }) => ({ default: Bar })));
However, the following isn't (understandably)
const Component = lazyImport(() => import('./Foo'), 'Foo');
any
The previous stackblitz didn't have @types/react
installed. This has been fixed.
--include-libs
No effect
This would affect almost all uses of react.lazy()
which is meant to work with default exports. If you try finding all references for Baz
, it won't resolve. Luckily, Knip still catches this one.
I don't think it's fair to make this the bar.
As per the previous discussion, Knip not catching this is fair. We're abstracting the behavior across multiple functions, so it's not clear that 'Foo'
is an export from ./Foo.tsx
. However, it is statically analyzable, just non-standard.
Here are some ideas for solutions that I think would work for most projects like this
import-list
optionThis one is a bit messy but it technically solves the problem by allowing the consumer to do the work of resolving the imports.
I can run a script which finds all the imports and which file they're imported. It might look something like
importList: {
// path it's imported from
'./foo/bar/baz.ts': {
// path it's importing
'./foo.tsx': [
// named imports
'Foo', 'Bar'
]
}
}
I would pass this option to Knip right before running and Knip would then keep this information in mind when resolving imports.
It would treat it the same as if it say import { Foo } from './Foo'
This one might be more perf-intensive, but it's opt-in.
For any file touched, run a script which defines the imports. Same as above, but doesn't force us to walk the tree ourselves.
Less fun since it'll probably miss a bunch of cases, but it would help get rid of the absolutely massive amount of false-positives reported by Knip currently.
If knip sees () => import('./Foo')
then include all named exports of Foo
as an import.
Pay now to fund the work behind this issue.
Get updates on progress being made.
Maintainer is rewarded once the issue is completed.
You're funding impactful open source efforts
You want to contribute to this effort
You want to get funding like this too