sindresorhus / eslint-plugin-unicorn

More than 100 powerful ESLint rules
MIT License
4.18k stars 362 forks source link

Import naming rule #6

Open jamestalmage opened 8 years ago

jamestalmage commented 8 years ago

I like the practice of assigning imports to variables that are camel-cased versions of the package name:

var objectAssign = require('object-assign');
var mapObj = require('map-obj');

We've been pretty consistent about using it in AVA, and I like knowing which package to lookup on npm without scrolling to the top (and I am disappointed when that doesn't work out).

There are exceptions though (perhaps so many it makes an enforcement rule impossible):

// shortening really long name
var throwsHelper = require('babel-plugin-throws-helper');

// reordering? maybe this only makes sense for packages with enforced prefixes.
var throwsHelperPlugin = require('babel-plugin-throws-helper');

// widely understood conventions
var _ = require('lodash');

Maybe the default should just be a warning?

Maybe there are just too many valid exceptions to make this worth it?

Maybe we could encourage a preferredVariableName section to package.json?

{
  "name": "lodash",
  "version": "x.x.x",
  "preferredVariableName": ["_", "lodash"]
}

As for handling ES2015 module imports: only apply to default imports.

jfmengels commented 8 years ago

I like the idea, but I think there are too many exceptions. I think simply enforcing camel-case would go a long way in the right direction.

jamestalmage commented 8 years ago

I think there are too many exceptions.

That was my concern as well. Still, it's a comment we leave on a lot of PR's. So a rule that got it right would be super helpful.

I think simply enforcing camel-case would go a long way in the right direction.

We already do that, and people still try to get creative with naming.


Maybe we could parse Readme.md from a given package and automatically allow users to follow the naming convention applied in the docs? That would cover lodash (and any well documented module).

For long names (anything over two words, or length X), maybe we only require you use (at a minimum) two words to create your camel case name, babel-plugin-ava-throws-helper, could then be called:

The second and third options above are pretty terrible, but nobody is going to pick those anyways.

jfmengels commented 8 years ago

Maybe we could parse Readme.md from a given package and automatically allow users to follow the naming convention applied in the docs?

That'd make your linting succeed or fail on the whims of the maintainer (If they change the name, or a bit more probable if they change the docs in a way that is not parsable). Sounds like a hell of a lot of trouble too (though I like the idea in itself).

I don't have a better idea yet though :/

jamestalmage commented 8 years ago

That'd make your linting succeed or fail on the whims of the maintainer

Good point.

We would always allow the camel casing of the name (so you are only subject to the whims of the maintainer when you break that pattern). The Readme parsing would only relax the rule by allowing additional alternatives.

sindresorhus commented 8 years ago

I really like the idea. I've been doing exactly this for a long while now and it makes imports easier to reason about. I don't think we should parse the readme though. It's just too fragile and convoluted. XO shouldn't suddenly fail because an author slightly changed the readme in a patch release.

For long names (anything over two words, or length X), maybe we only require you use (at a minimum) two words to create your camel case name, babel-plugin-ava-throws-helper, could then be called

Yeah, I like that. Seems like a good compromise.

I also think we could add common conventions, like _ for Lodash/Underscore and $ for jQuery. That should cover most. And an option to add custom aliases.

About making it a warning. That might be sensible at first, but the rule is never going to be very useful as a warning, as warning won't fail Travis, and pretty much no one runs npm test locally...

jfmengels commented 8 years ago

For long names (anything over two words, or length X), maybe we only require you use (at a minimum) two words to create your camel case name, babel-plugin-ava-throws-helper, could then be called...

I'm not sure how it will turn out, but we could try it in a few projects and see if it's too painful to respect. Though I could see myself wanting this too for instance (which wouldn't pass with what you proposed)

var foo = require('babel-plugin-foo');
var stuff = require('babel-plugin-stuff');

I think we should allow little changes to the name too, like prepending/appending small stuff, so that it can be assigned to a different variable with the right name later. Example:

var curry = require('curry');
var _objectAssign = require('object-assign');

var objectAssign = curry(_objetAssign);

We could maybe only allow it an other variable exists with the name we would have wanted it to have?

// Allowed
var curry = require('curry');
var _objectAssign = require('object-assign');
var objectAssign = curry(_objetAssign);

// Not allowed
var curry = require('curry');
var _objectAssign = require('object-assign');

Don't know if it's a good idea.

:+1: with @sindresorhus with warnings. If they are not errors, they will just pile on and pollute the console.

no one runs npm test locally

Really? Hah, I do it all the time. Surprising.