webpro-nl / knip

✂️ Find unused files, dependencies and exports in your JavaScript and TypeScript projects. Knip it before you ship it!
https://knip.dev
ISC License
6.61k stars 152 forks source link

How to ignore unlisted dependencies? #159

Closed gajus closed 1 year ago

gajus commented 1 year ago
Unlisted dependencies (3)
@typescript-eslint/parser          .eslintrc.js
eslint-config-canonical            .eslintrc.js
eslint-import-resolver-typescript  .eslintrc.js

It looks like it infers that the plugin is in use based on which rules are mentioned:

const ruleOverrides = {
  '@typescript-eslint/dot-notation': 0,
  '@typescript-eslint/naming-convention': 0,
  '@typescript-eslint/no-misused-promises': 0,
  '@typescript-eslint/no-redeclare': 0,
  '@typescript-eslint/no-unnecessary-condition': 0,
  '@typescript-eslint/prefer-nullish-coalescing': 0,
  '@typescript-eslint/prefer-optional-chain': 0,
  '@typescript-eslint/prefer-string-starts-ends-with': 0,
  '@typescript-eslint/promise-function-async': 0,
  'canonical/filename-match-regex': 0,
  'canonical/id-match': 0,
  'canonical/prefer-import-alias': [

This is smart. However, in cases like eslint-config-canonical, the config itself has a dependency on the plugins, and this becomes and undesirable.

gajus commented 1 year ago

https://github.com/webpro/knip/blob/main/docs/handling-issues.md#unlisted-dependencies

Suggests to add the dependency directly, but in this case it would be undesirable.

gajus commented 1 year ago

I ended up adding .eslintrc.js to ignore and that got rid of the "Unlisted dependencies" error, but introduced:

Unused devDependencies (2)
@contra/eslint-config  package.json
eslint-plugin-relay    package.json

At least I can ignore these, but wish these was a better solution.

webpro commented 1 year ago

Knip doesn't use rules to infer dependencies (it looks at extends plugins, parser and settings, and also in overrides for the same).

But looking at https://github.com/gajus/eslint-config-canonical/blob/main/configurations/eslintrc.js, this looks like a bug in Knip. Will look into it soon!

webpro commented 1 year ago

Not sure if I'm trying the right thing, but in that repo, either one of these configs (knip.json) seem to work well:

{
  "entry": ["compare/*.js"],
  "eslint": ["*.js"]
}
{
  "workspaces": {
    ".": {
      "eslint": ["*.js"]
    },
    "compare": {}
  }
}
gajus commented 1 year ago

The error is in a repository that consumes eslint-config-canonical as a dependency.

webpro commented 1 year ago

Then I wonder why it is reported as unlisted. Got a rep(r)o? :)

gajus commented 1 year ago

If helpful, this is the entire config:

const path = require('node:path');

const ruleOverrides = {
  '@typescript-eslint/dot-notation': 0,
  '@typescript-eslint/naming-convention': 0,
  '@typescript-eslint/no-misused-promises': 0,
  '@typescript-eslint/no-redeclare': 0,
  '@typescript-eslint/no-unnecessary-condition': 0,
  '@typescript-eslint/prefer-nullish-coalescing': 0,
  '@typescript-eslint/prefer-optional-chain': 0,
  '@typescript-eslint/prefer-string-starts-ends-with': 0,
  '@typescript-eslint/promise-function-async': 0,
  'canonical/filename-match-regex': 0,
  'canonical/id-match': 0,
  'canonical/prefer-import-alias': [
    2,
    {
      aliases: [
        {
          alias: '@/',
          matchParent: path.resolve(__dirname, 'src/components'),
          matchPath: '^src\\/',
        },
        {
          alias: '@/',
          matchParent: path.resolve(__dirname, 'src'),
          matchPath: '^src\\/',
        },
        {
          alias: '@/',
          matchPath: '^src\\/',
          maxRelativeDepth: 2,
        },
      ],
    },
  ],
  'canonical/prefer-use-mount': 2,
  'default-case': 0,
  'default-case-last': 0,
  'import/extensions': 0,
  'import/no-named-default': 0,
  'jsx-a11y/mouse-events-have-key-events': 0,
  'no-restricted-globals': [
    2,
    {
      message: 'Please use localStorage from @/services/storage',
      name: 'localStorage',
    },
    {
      message: 'Please use sessionStorage from @/services/storage',
      name: 'sessionStorage',
    },
  ],
  'no-restricted-imports': [
    2,
    {
      paths: [
        {
          importNames: ['default'],
          message: 'Please import a named export.',
          name: 'react',
        },
        {
          importNames: ['graphql'],
          message: 'Please use graphql from relay-runtime.',
          name: 'react-relay',
        },
        {
          importNames: ['useFlags'],
          message: 'Please use useFeaturedFlags from @/hooks.',
          name: 'launchdarkly-react-client-sdk',
        },
        {
          importNames: ['motion', 'm'],
          message: 'Please use animated from @/components/Primitives',
          name: 'framer-motion',
        },
        {
          importNames: ['useMutation'],
          message: 'Please use useContraMutation from @/hooks.',
          name: 'react-relay',
        },
        {
          message: 'Please use zod for validation.',
          name: 'yup',
        },
        {
          importNames: ['captureException'],
          message: 'Please use captureException from @/utilities.',
          name: '@sentry/react',
        },
        {
          importNames: ['default'],
          message: 'Please use dayjs from @/utilities/dayjs.',
          name: 'dayjs',
        },
      ],
    },
  ],
  'no-restricted-properties': [
    2,
    {
      message: 'Please use localStorage from @/services/storage',
      object: 'window',
      property: 'localStorage',
    },
    {
      message: 'Please use sessionStorage from @/services/storage',
      object: 'window',
      property: 'sessionStorage',
    },
    {
      message: 'Use fast-safe-stringify',
      object: 'JSON',
      property: 'stringify',
    },
    // TODO [2023-04-23]: needs to be re-enabled
    // {
    //   message: 'Please use the new color system accessed via theme.colorsV2',
    //   object: 'theme',
    //   property: 'colors',
    // },
  ],
  'no-restricted-syntax': ['error', 'Literal[value=/^@validate$/i]'],
  'no-warning-comments': 0,
  'node/no-process-env': 0,
  'node/no-sync': 0,
  'prefer-destructuring': 0,
  'prefer-object-spread': 0,
  'promise/prefer-await-to-then': 0,
  'react/forbid-component-props': 0,
  'react/hook-use-state': 0,
  'react/iframe-missing-sandbox': 0,
  'react/no-array-index-key': 0,
  'react/no-invalid-html-attribute': 0,
  'react/no-unstable-nested-components': 0,
  'react/no-unused-prop-types': 0,
  'react/prop-types': 0,
  'regexp/no-obscure-range': 0,
  'regexp/no-super-linear-backtracking': 0,
  'regexp/no-unused-capturing-group': 0,
  'relay/compat-uses-vars': 2,
  'relay/function-required-argument': 2,
  'relay/graphql-naming': 2,
  'relay/graphql-syntax': 2,
  'relay/hook-required-argument': 2,
  'relay/must-colocate-fragment-spreads': 0,
  'relay/no-future-added-value': 0,
  'relay/unused-fields': 2,
  'require-unicode-regexp': 0,
  'unicorn/import-index': 0,
  'unicorn/no-array-callback-reference': 0,
  'unicorn/no-array-for-each': 0,
  'unicorn/no-array-reduce': 0,
  'unicorn/no-thenable': 0,
  'unicorn/no-unsafe-regex': 0,
  'unicorn/no-useless-switch-case': 0,
  'unicorn/prefer-code-point': 0,
  'unicorn/prefer-node-protocol': 0,
  'vitest/max-nested-describe': 0,
};

module.exports = {
  extends: ['@contra/eslint-config'],
  ignorePatterns: ['/dist', '**/__generated__'],
  overrides: [
    {
      files: '*.{ts,tsx}',
      rules: {
        'canonical/no-barrel-import': 2,
        ...ruleOverrides,
      },
      settings: {
        'import/parsers': {
          '@typescript-eslint/parser': ['.ts', '.tsx'],
        },
        'import/resolver': {
          typescript: {
            project: path.resolve(__dirname, 'tsconfig.json'),
          },
        },
      },
    },
    {
      extends: ['canonical/jsx-a11y'],
      files: '*.tsx',
      rules: ruleOverrides,
    },
    {
      files: './tests/**/*',
      rules: {
        'import/no-cycle': 0,
      },
    },
  ],
  plugins: ['relay'],
  root: true,
};
webpro commented 1 year ago

Given only that config, Knip finds these dependencies (rules are ignored):

    '@contra/eslint-config',
    'eslint-plugin-relay',
    '@typescript-eslint/parser',
    'eslint-import-resolver-typescript',
    'eslint-config-canonical',
webpro commented 1 year ago

Normally the dependencies would be direct or peer dependencies. Not sure how Knip could make an exception here without affecting other setups.

webpro commented 1 year ago

Going to close this issue. Since the dependencies are "directly" referenced in the ESLint config, Knip can only assume they should be listed in package.json.

Fixing this issue would mean transitive dependencies are OK to directly depend on, and I prefer not to go that way.

gajus commented 1 year ago

That's fair.

I would still consider ignoreUnlisted configuration for cases where it is desirable to support transitive dependencies.

I am trying to restructure out project to avoid this altogether.

davidarny commented 11 months ago

@webpro I have a same issue. 

I'm sharing config files across multiple repos as an npm library, which itself contains eslint configuration and dependencies.

Typically eslint config of an app consuming this library looks like this:

const merge = require("deepmerge");
const { createEslintConfig } = require("@omnic/widget-config");

const rootConfig = createEslintConfig();

module.exports = merge(rootConfig, {
  root: true,
  env: { node: true },
  rules: { "@typescript-eslint/no-var-requires": "off" },
});

And here is my output

Fixing this issue would mean transitive dependencies are OK to directly depend on, and I prefer not to go that way.

Isn't it possible to optionally enable transitive dependencies check? It would be very helpful in case of using Knip for module federations for example, cause they usually have some shared configs/ui-kits, and duplicating all the dependencies is not a way

P.S. Also can't figure out how to deal with these .svg imports (I'm using vite + svgr plugin)

webpro commented 11 months ago

Isn't it possible to optionally enable transitive dependencies check?

This is a common yet non-trivial issue to solve. It's on my radar.

P.S. Also can't figure out how to deal with these .svg imports (I'm using vite + svgr plugin)

This is usually worked around by using relative imports (e.g. ./src/icon.svg).