-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
no-extraneous-dependencies
: add an option to allow specific package names to be in different categories
#903
Comments
Right.
import type {AnyReactElement, ReactChildren} from 'react-flow-types';
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 |
Linting should be done in dev, not production, so all dev dependencies would always be present. |
@ljharb Not sure I got what you meant. But one of the purpose of
|
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) |
But variable name can be absolutely random, not everybody uses NODE_ENV. 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 |
@ljharb hey, any chances revisiting this? we could make a PR for that. I think, that |
It definitely might make sense for no-extraneous-deps to use the import/ignore setting. |
I think, like |
Is there any progress on this? 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? |
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. |
@kalbert312 can't agree more |
for example using this packages, validatorjs/validator.js ESLint: v7.26.0 |
@ljharb I'm surprised that no one has raised this use case... When writing for AWS Lambda, it's the standard practice to place // eslint-disable-next-line import/no-extraneous-dependencies
export * from 'aws-sdk' (Or the equivalent Being able to tell the rule to ignore |
@shellscape it should instead be kept in deps, but omitted from whatever zip you send to AWS. |
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. |
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. |
no-extraneous-dependencies
: add an option to allow specific package names to be in different categories
+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
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.
Those types are never needed in the Being able to allow packages in devDependencies by a string match and/or regular expression would solve a few headaches here. |
Just to clarify and support the proposal, the electron package is added by electron-builder, so it should be listed as a |
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'signore
regex setting.This could also be a potential solution for #496
The text was updated successfully, but these errors were encountered: