Proposal: `default-import-match-filename`

[gajus]

I often see things such as:

import parseDate from './types/date';
import float from './types/parseFloat';

The disparity between the default import name (parseDate) and file name (date) often indicates that one or the other is named inappropriately. In the first case, thats the file name. In the second case, thats the local variable name.

Valid code:

import parseFloat from './types/parseFloat';
import parseDate from './types/parseDate';

Invalid code:

import parseDate from './types/date';
import float from './types/parseFloat';

There is no way to enforce a right name, e.g. user could just as well end up with:

import date from './types/date';
import float from './types/float';

However, I would argue that these issues are easy to notice in the code. Having this rule would prevent lazy developers from using a better variable name in a local file and ignore the need to rename the file and other references.

Furthermore, this enables easier code inspection (because it is easy to recognise what a variable represents when it is being used consistently between many files).

[benmosher]

Makes sense to me. Probably needs a little more detail/scope to build out.

How would you propose packages be handled? I.E.

import React from 'react'

is idiomatic, but not sure how one would divine this from just the import path.

[gajus]

I would propose to have a separate rule for overseeing module imports and separate rule for importing files, i.e. default-import-match-module and default-import-match-filename. I will leave the question of react case to whoever becomes a proponent of the default-import-match-module rule.

[benmosher]

Got it, so this rule's scope would be only internal relative imports, as you've described?

[jfmengels]

We've had a similar discussion on eslint-plugin-xo sindresorhus/eslint-plugin-unicorn#6, though we only spoke about modules (but I think they should probably be considered the same).

I have trouble seeing this as something that can be generalized enough. For example, how would you enforce / generalize the following?

import parseFloat from './parse/float';
import parseDate from './parse/date';
import formatFloat from './format/float';
import formatDate from './format/date';

or

import closeToFoo from './foo';

function foo(...) { ... }

export foo;

cc @jamestalmage @sindresorhus

[benmosher]

I would think that such a folder layout simply couldn't use the rule that @gajus has described.

It would be workable as he has described if it stays as simple as "default import identifiers should match an imported file's name".

[gajus]

First of all, if you have:

import parseFloat from './parse/float';
import parseDate from './parse/date';
import formatFloat from './format/float';
import formatDate from './format/date';

This is specifically the design flaw that I am describing.

What is the reason you are naming your variables different than the files? In this specific context, it is because variable name makes more sense in the code than simply "float" or "date". Then what is the reason you are naming a file containing "parseFloat" function as "float"? It makes no sense. When considering a file name, file path should have no weight. With this rule, you would be forced to restructure your code as:

import parseFloat from './parse/parseFloat';
import parseDate from './parse/parseDate';
import formatFloat from './format/formatFloat';
import formatDate from './format/formatDate';

(or choose not to use the rule).

[jfmengels]

When considering file name, file path should have no weight

Hm, I'd like to disagree. I don't see why the file path should have no weight. If I have my float handler in a parse/ folder, then I'm just repeating myself, and I don't see the point. I also often have a folder with an index.js file (like foo/bar/index.js), so that I can just do import bar from './foo/bar'; and "hide" the internal workings in other files. It's not per se in conflict with the rules, just pointing out that I need the file path to infer what foo/bar/index.js does, while being a valid use IMO. This pattern has worked fine for me thus far.
(if you guys feel like I'm taking the conversion somewhere it shouldn't, I'll be happy to discuss this somewhere else).

(or choose not to use the rule)

That is of course an acceptable solution ;) I have no issue with this issue being in the repo at all. I just have a concern that if you generalize this to external modules (it is similar, but I guess that it is done for different purposes here, so one could argue one way or the other IMO), then you'd have issues all over the place for handling it one way or another, have exceptions, etc., and that could be a huge burden on the maintainer.

[gajus]

Hm, I'd like to disagree. I don't see why the file path should have no weight. If I have my float handler in a parse/ folder, then I'm just repeating myself, and I don't see the point. I also often have a folder with an index.js file (like foo/bar/index.js), so that I can just do import bar from './foo/bar';

I can think of many examples. However, style is one of those things that it is easy to find arguments in both directions.

To quickly summarize my position:

1. You want to name files the same way you will name the variable that will represent the file contents. The reason: it will promote consistent name of a resource throughout the code base.

2. A path of the file will not always be visible. Consider example:

   ./formatters/hour.js
   ./formatters/minute.js
   ./formatters/time.js
   

   Now in ./time.js you want to import ./hour.js and ./minute.js.

   You cannot tell what ./hour and ./minute only by scanning the code (such is often the case when reviewing code fragments in PRs)

There are a few more arguments (such as promoting code re-usability), though this is getting off-topic.

This rule is opt-in and it will benefit developers who prefer explicit code naming conventions.

[mizchi]

I think we need default-export-match-filename too.

Example

// MyComponent.js
// pass
export default function MyComponent() { return ... }
// fail
export default function NotMyComponent() { return ... }

[ljqx]

I think this rule (both export & import) can be configured by

1. what to match

• base file name
• full path from project root (module id?)

2. how to match

• exact match (as Airbnb style)
• filename/module id kebab case (friendly to Windows users, consistent with npm)
• ...

3. how to treat index.js

• no special handling
• the folder name is used as base file name or full path end

Then we can fit all requirements from different projects above, and leave the option to users.

For

import parseFloat from './parse/float';
import parseDate from './parse/date';
import formatFloat from './format/float';
import formatDate from './format/date';

It's valid for "full path" match, but not "base file name" match.

[ljharb]

I'm not sure about your latter example - is parseFloat valid if the path is ./a/b/c/parse/float?

I think the rule should only ever use the base filename, or the base dirname if it's an index (the latter could be configurable, but should be on by default).

The rule would need to handle the most common cases; it's not practical for it to attempt to cover every conceivable pattern.

[ljqx]

@ljharb
parseFloat is not valid when the path is ./a/b/c/parse/float for either “module specifier" match or "base filename" match. There will be only one parseFloat for the whole project, I think it's ok that it's at the top level of project as parseFloat.js (base filename or module specifier) or parse/float.js(module specifier) for grouping.

Matching base filename is not always good, especially for big project which has complicated features which is grouped by folders. As an example, there is one file named feature/subFeature/subSubFeature/Editor, we want it have a good name for readability/debuggability, then should we change the file to feature/subFeature/subSubFeature/FeatureSubFeatureSubSubFeatureEditor? Why duplicate the path again? That's same for parse/float too: parseFloat shows up with parse/float when importing import parseFloat from 'parse/float', later parseFloat is self-describing by its name, every code piece is readable, and no duplicate in path.

And, ./a/b/c/parse/float is not covered here :).

There maybe another configuration for all exceptions like import _ from 'lodash'.