False positive on import * as S from './file'

[noahbald]

Not sure if this is going to be in scope for Knip as it seems like an edge case, but the following pattern is causing a false positive.

// File with many exports
import styled from '@emotion/styled';

export const h1 = styled.h1`...`;

export const h2 = styled.h2`...`;

// ...

// File that imports all exports
import React from 'react';
import * as S from './styles';

const Heading = ({ as, ...props }) => {
    const Element = as ? S[as] || S.h1 : S.h1;
    return <Element {...props} />;
};
export default Heading;

[webpro]

I guess the false positive is h2?

Yeah I don't think there's a way for Knip to infer that. If the types are known Knip should be able to infer, but as in the example isn't typed at all? But even if it were I expect it won't help Knip enough.

Example of a pattern that Knip is able to work with: https://github.com/webpro/knip/blob/main/packages/knip/fixtures/re-exports-ns-type/index.ts

[noahbald]

I would expect as to be typeof S or maybe string - in either case if we’re using a star import and accessing that via a computed key, would knip be able to mark everything under * as S as a used key export?

For me, the false positive is h1, h2, and any other export in the file

[webpro]

I would expect as to be typeof S or maybe string

From your example there's no way to infer that.

in either case if we’re using a star import and accessing that via a computed key, would knip be able to mark everything under * as S as a used key export?

No. Also see https://knip.dev/blog/knip-v5

For me, the false positive is h1, h2, and any other export in the file

From your example, I would expect S.h1 to be referenced/used. So that might be a bug. Regardless, that won't help you much here I'm afraid.

[erheron]

in either case if we’re using a star import and accessing that via a computed key, would knip be able to mark everything under * as S as a used key export?

No. Also see https://knip.dev/blog/knip-v5

Hmm I think some people might want to "turn on" counting star imports as "everything is used". Consider something like this:

/// file assets.tsx
export { default as cat } from 'myproject/assets/pictures/at.png';
...
export { default as dog } from 'myproject/assets/pictures/dog.png';


/// file Picture.tsx
import * as pictures from '../assets.tsx';

export type PictureToken = keyof typeof pictures;

interface Props {
   pictureToken: PictureToken;
}


export const Picture = (props : Props) => {
  /// some code which abstracts getting actual asset from the dictionary, resizing, etc. etc.
}

And note that it might actually be impossible to detect in compile-time if e.g. a dog asset is unused, if one is getting their tokens from server (so it could be any value). So should we even try?

[webpro]

counting star imports as "everything is used"

Not sure how the intermediate keyof typeof pictures makes Knip behave, but in general Knip tries to infer that. Also see fixtures for examples, e.g. https://github.com/webpro/knip/blob/main/packages/knip/fixtures/imports-namespace/index.ts where members of NS3, NS4, NS5, NS6 are marked as used (because no members are accessed individually).

And note that it might actually be impossible to detect in compile-time if e.g. a dog asset is unused, if one is getting their tokens from server (so it could be any value). So should we even try?

Knip does static analysis only, it does indeed not run code and check values dynamically.

[erheron]

And note that it might actually be impossible to detect in compile-time if e.g. a dog asset is unused, if one is getting their tokens from server (so it could be any value). So should we even try?

Knip does static analysis only, it does indeed not run code and check values dynamically.

Hmm I think there's a little misunderstanding. I know knip doesn't check values dynamically 😄
What I wanted to say is that in an example above (one with Picture component) to me, all exports from assets.tsx should be considered used (given there is a code like below somewhere in the codebase)

...
const pictureToken = getPictureTokenFromServer(); // dynamic value
return (<Picture pictureToken={pictureToken} />);
...

[webpro]

Gotcha. That's "too dynamic" indeed. Knip v3 might deal with this better, but it got too slow.

TypeScript's language service findReferences function might deal with this fine, but I moved away from it, for the same reasons.

[webpro]

🚀 This issue has been resolved in v5.3.0. See Release 5.3.0 for release notes.

[webpro]

For the examples in this thread: with proper types, Knip should find references. Played a bit with some scenarios and added to the fixtures: https://github.com/webpro/knip/blob/main/packages/knip/fixtures/re-exports-ns-type/index.tsx

Specific to the as property: if it's typed properly the exports are all referenced.

This functionality hasn't changed in the latest release, I've just added the fixture to confirm it's working as I would expect.