import-js / eslint-plugin-import

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

no-extraneous-dependencies: Optionally do not tell the user to use npm #1376

Open NickHeiner opened 5 years ago

NickHeiner commented 5 years ago

My team uses yarn. The error message for no-extraneous-dependencies says:

error  'falcor' should be listed in the project's dependencies. Run 'npm i -S falcor' to add it  import/no-extraneous-dependencies

I'd rather either omit the second sentence entirely, or add an option to tell devs to use yarn. Otherwise, it could create confusion on my team.

ljharb commented 5 years ago

npm is the default; any user of yarn always has to implicitly know how to convert npm commands into yarn commands - that's just the cost of using yarn.

NickHeiner commented 5 years ago

I think that's an inflexible and user-unfriendly approach; my project has some contributors who are less familiar with the Node ecosystem. And more broadly, I don't like giving anyone an error message that's explicitly the wrong thing to do. So, as-is, I can't use this project.

What if I submitted a PR allowing the user to override the error message entirely via rule config?

ljharb commented 5 years ago

I might except an option to override the install command specifically - but in general, people unfamliar with the node ecosystem can't really avoid needing to learn it in order to contribute. Also, if your goal is "user friendly", then you'd use npm, since it's the default.

NickHeiner commented 5 years ago

Also, if your goal is "user friendly", then you'd use npm, since it's the default.

I see your point, but I'm not going to derail this into an npm v. yarn debate. :smile:

I might except an option to override the install command specifically

That would be perfect. For instance, I'd also override it in my case to be the workspace-aware command.

ljharb commented 5 years ago

Seems reasonable. If there’s more than one rule that has similar messaging, let’s put it in settings instead of rule options.

NickHeiner commented 5 years ago

This is what I've done as a workaround. :smile:

const originalRule = require('eslint-plugin-import/lib/rules/no-extraneous-dependencies');
const _ = require('lodash');

const regex = /'([@/a-z0-9-]*)' should be listed/i;

module.exports = {
    ...originalRule,
    create: (context) => {
        const interceptingContext = _.cloneDeep(context);
        interceptingContext.report = (...args) => {
            const [node, message] = args;
            const match = message.match(regex);
            if (match) {
                const packageName = match[1];
                const newMessage = `Add '${packageName}' to this package's package.json.`;
                context.report(node, newMessage);
                return;
            }
            return context.report(...args);
        };

        return originalRule.create(interceptingContext);
    }
};
ljharb commented 5 years ago

You could also use eslint-rule-composer.

NickHeiner commented 5 years ago

Ha! Neat. I didn't know about that.