Add no-namespace rule

[radekbenkel]

no-namespace

Reports if namespace import is used.

Rule Details

Valid:

import defaultExport from './foo'
import { a, b }  from './bar'
import defaultExport, { a, b }  from './foobar'

...whereas here imports will be reported:

import * as foo from 'foo';
import defaultExport, * as foo from 'foo';

Motivation:

One should only import names which are needed in current scope, nothing more.

Additonally, users of tools like rollup.js or webpack 2.0 which support tree shaking would benefit in a smaller bundle size because unused exports that are not imported can be eliminated.

If all names are required in addition to separate named exports, developer might export default object with all required members:

export const FOO = 1;
export const BAR = 2;

export default {
    FOO,
    BAR
}

[benmosher]

FWIW: if there are no dynamic references to the namespace, you can still do tree-shaking. I don't believe there should be any performance impact to using a namespace import vs. individual names/default.

I am up for including this rule, but I think the motivation section of the doc should be revised to remove that language and it should be listed in the README under the Style guide list of rules?

[lo1tuma]

@benmosher There is not always a way to statically analyze all references, for example:

import * as foo from './foo';
import thirdPartyLibrary from 'third-party';

thirdPartyLibrary.doSomething(foo);

[benmosher]

@lo1tuma fair enough. That would be easy enough to lint explicitly, though. I can imagine some more nuanced enforce-tree-shaking rule that reports anything that compromises static analysis:

• passing namespaces as a parameter (as you have shown)
• dynamic references (foo[someKey])
• etc.

I just don't want folks getting the idea that imported namespaces themselves compromise analysis, which I think the current state of the "Motivation" section in this PR pretty strongly implies.

Also "One should only import names which are needed in current scope, nothing more." is a pretty opinionated statement, IMO.

So I would be inclined to turn "Motivation" into

## When Not To Use It
If you want to use namespaces, you don't want to use this rule.

[lo1tuma]

@benmosher Agreed, it makes sense to have a separate rule which explicitly enforces patterns that can be optimized by tree-shaking.

[radekbenkel]

OK, I've updated rule docs and moved rule to the style guide section in README.md.

Thanks for your feedback guys :)

[benmosher]

Looks good! Thanks!

[benmosher]

Oh actually, could you add a blurb under the Unreleased section of CHANGELOG.md?

[radekbenkel]

@benmosher no problem, done.

[benmosher]

Awesome, thanks!

[gajus]

@singles Okay. The rule is built on a premise that discouraging use of import all will enable tree-shaking. However, file size is not the only concern. For the most part, importing just certain properties makes the code easier to read, e.g.

import {FOO, BAR} from './config';

// ...
// Do something with FOO, BAR

is a lot easier to scan than

import config from './config';

// ...
// Do something with config.FOO, config.BAR

However, at the same time, I might need to import the entire config, e.g. to serialise the config.

This rule encourages to use export default whenever there is at least one instance in code where there is need to import the entire config. In effect, promoting use of the pattern in the latter example.

[radekbenkel]

@gajus tree shaking is one of the examples. Also, in our team we're considering importing only needed stuff as good practice.

When namespace imports are disallowed, author of library/module can decide, if he want's to expose whole object or not.

Then

export const FOO = 123;
export const BAR = 456;

allows you to import only specific members (and whole module using *), but if library author decides that they would also like to export whole stuff, they can always do:

export const FOO = 123;
export const BAR = 456;

export default { FOO, BAR };

And of course, you can always use //eslint-disable

[gajus]

@singles that is a good example:

export const FOO = 123;
export const BAR = 456;

export default { FOO, BAR };

Thank you. This made me change the setting for no-namespace from 0 to 2. : )