import-js / eslint-plugin-import

ESLint plugin with rules that help validate proper imports.
MIT License
5.55k stars 1.57k forks source link

`no-extraneous-dependencies`: add an option to allow specific package names to be in different categories #903

Open lukechilds opened 7 years ago

lukechilds commented 7 years ago

It would be great if the import/no-extraneous-dependencies rule could be ignored project wide but only for certain dependencies.

import/no-unresolved already has this functionality with it's ignore regex setting.

This could also be a potential solution for https://github.com/benmosher/eslint-plugin-import/issues/496

klimashkin commented 7 years ago

Right. It is really needed in two cases:

  1. In case of Flow. For instance in typing react component.

    import type {AnyReactElement, ReactChildren} from 'react-flow-types';

    react-flow-types are obviously in devDependencies, it never goes to production

  2. In case of __DEV__ environment variable (or process.env.development or whatever) when we dynamcally require packages from devDependencies for development only

    if (process.env.NODE_ENV !== 'production' && __WHY_UPDATE__ === true) {
    const {whyDidYouUpdate} = require('why-did-you-update'); 
    const RedBox = require('redbox-react');
    
    require('expose-loader?Perf!react-addons-perf');
    }

Without having ignore option in no-extraneous-dependencies rule, I have to add // eslint-disable-line import/no-extraneous-dependencies on such lines. But in case of Flow it hardly possible, because requires too many such lines, in all react components files

ljharb commented 7 years ago

Linting should be done in dev, not production, so all dev dependencies would always be present.

klimashkin commented 7 years ago

@ljharb Not sure I got what you meant. Of course linting happens in dev.

But one of the purpose of no-extraneous-dependencies rule is to restrict accidental usage of packages from devDependencies in your application that is built with webpack for browser. In previous message I described why it should allow exceptions - when developer knows that he uses package from devDependencies in built application but only in development mode. Otherwise developer needs to put, for instance, why-did-you-update into dependencies, which is wrong.

ljharb commented 7 years ago

Ah, I think I understand.

Would it perhaps make more sense to have an option that skips checking dev deps inside NODE_ENV === production blocks, or similar? An ignore list is just as hard to maintain and get right as individual ignore comments are (which is what I'd suggest in the meantime)

klimashkin commented 7 years ago

But variable name can be absolutely random, not everybody uses NODE_ENV. And it would not solve imports of flow types, they are not conditional.

I don't think ignore list for that rule is harder to maintain than for others. We have only about 5-6 packages to ignore among our 160+ items in package.json

panrafal commented 7 years ago

@ljharb hey, any chances revisiting this? we could make a PR for that. In our use case, we have special imports transformed later by babel (translations to be precise). We have them ignored in import/ignore and import/no-unresolved.

I think, that no-extraneous-dependencies should either use import/ignore setting, or have their own ignore (or use the global setting as a fallback).

ljharb commented 7 years ago

It definitely might make sense for no-extraneous-deps to use the import/ignore setting.

panrafal commented 7 years ago

@ljharb I've made a PR in #943

benmosher commented 7 years ago

I think, like no-unresolved, it may make sense for this rule to have its own ignore setting instead of using the global import/ignore, as that setting is more intended to avoid parsing non-JS "imports" in build pipelines with lots of transpiling and/or plugins to make that possible (i.e. CSS modules).

EvertEt commented 5 years ago

Is there any progress on this? We currently have a seperate repo containing our typescript types as ES modules. This package is in the devDeps as the types get removed during compilation and thus not needed during running. We found it impossible to ignore this single package (using core-modules or ignore).

If there is no progress, is there already an idea what the ideal solution would be? Would it be a own ignore setting based on glob or just package names?

kalbert312 commented 5 years ago

This would be nice to have for TypeScript packages that only export types. Types are a devDependency in a leaf project but will never make it to production.

tjx666 commented 4 years ago

@kalbert312 can't agree more

latipun7 commented 3 years ago

for example using this packages, validatorjs/validator.js

validator error types

ESLint: v7.26.0 plugin-import: v2.23.0

shellscape commented 3 years ago

@ljharb I'm surprised that no one has raised this use case...

When writing for AWS Lambda, it's the standard practice to place aws-sdk in devDependencies as this module is automatically injected into each running lambda function. Including the package in dependencies (and thus the deployment zip for lambda) almost always inflates the ZIP beyond the size limits that lambda allows. As such, anywhere that a thing is imported from aws-sdk we get a flood of ESLint warnings/errors. There's no way to disable this unless we modify the rule's parameters to account for every file that aws-sdk is imported in. And that'd be a bit silly. The workaround is also a bit silly; creating a local sdk.ts (or equivalent sdk.js) which contains only:

// eslint-disable-next-line import/no-extraneous-dependencies
export * from 'aws-sdk'

(Or the equivalent module.exports for commonjs)

Being able to tell the rule to ignore aws-sdk is safe for this use case, and sure would avoid a lot of leg work for users.

ljharb commented 3 years ago

@shellscape it should instead be kept in deps, but omitted from whatever zip you send to AWS.

shellscape commented 3 years ago

I disagree - that's some exceptional buck-passing. Build complexity increase isn't worth the price of linting, especially when there's a reasonable feature waiting for the linting tool. The cost of failure is much higher for deployment than it is for this linting extension.

ljharb commented 3 years ago

i think I can see an argument for an option specifying that a specific package must be in devDependencies, even if all other deps in the file are in regular dependencies. That would allow for cases like AWS, electron, etc, without allowing for all the problematic things an ignore option would allow.

regexj commented 1 year ago

+1 on this.

I'm running into this error and finding it frustrating I cannot tell the no-extraneous-dependencies which packages I allow to be devDependencies.

In our setup we have a turbo-monorepo. In the packages folder we have a common types with sub directories according to the different workspaces. When they are included in workspaces the package.json looks like this:

"devDependencies": {
  "@project-types/name": "workspace:*",
}

Then elsewhere we include the types frontend and backend and have to slap an ignore no-extraneous-dependencies comments all over the place. E.g.

// eslint-disable-next-line import/no-extraneous-dependencies
import {
  Details,
  BrandGet,
  BrandLocation,
  BrandMedia,
  SchemaNames,
} from '@project-types/name';

Those types are never needed in the dependencies. Like the comment above we're also using aws-sdk in a few places, same again.

Being able to allow packages in devDependencies by a string match and/or regular expression would solve a few headaches here.