Suggestion: `require-namespace-imports`

[OliverJAsh]

For a given subset of modules, we would like to ensure that all imports for these modules use namespaces (aka not default or named imports).

Rationale

Inside of a module, sometimes we like to name things contextually according to the name of the module. This means that when these values are imported, they may appear ambiguous if they are not prefixed with a namespace. For example:

// ./query-params.ts

/** Get a query param */
export const get = (param: string) => { /* … */ }

// ./main.ts

import { get } from './query-params';

// 100 lines or so later…
// when skimming the code, it's not clear what `get` means?!
const result = get('foo');

Furthermore, we may need to import from other modules with named exports under the same name, e.g. in this example we may also need to import get from lodash.

// ./main.ts

// Naming conflicts
import { get } from './query-params';
import { get } from 'lodash-es';

(We see these naming conflicts happening more frequently now that many libraries are moving from instance methods to pipeable operators (e.g. RxJS, fp-ts, idb-keyval, Fluture, date-fns) because they have better bundling performance.)

If we use namespace imports, the names are no longer ambiguous and we no longer have naming conflicts:

// ./main.ts

import * as queryParams from './query-params';
import * as _ from 'lodash-es';

const result = queryParams.get('foo');

const result2 = _.get(/* … */);

Proof of concept

Here's how we're currently doing this:

// .eslintplugin.js

// This file allows us to define our own ESLint rules. It is powered by
// https://github.com/taskworld/eslint-plugin-local.

// Possible improvements:
// - rather than saying "has some child that is `ImportSpecifier` or `ImportDefaultSpecifier`", it
//   would be more explicit to say "has some specifier that is not `ImportNamespaceSpecifier`", but
//   I couldn't find a way to do this.
// - select whole import when match is found, rather than every child/specifier.
// - support for relative paths
/** @returns {import('eslint').Rule.RuleModule} */
const requireNamespaceImports = (() => {
  /** @param importPaths {string[]} */
  const createSelector = importPaths =>
    `:matches(${importPaths
      .map(importPath => `ImportDeclaration[source.value='${importPath}']`)
      .join(', ')}) :matches(ImportSpecifier, ImportDefaultSpecifier)`;

  return {
    create: context => {
      const [importPaths] = context.options;
      const selector = createSelector(importPaths);
      return {
        [selector]: node => {
          context.report({
            node,
            message: 'Only namespace imports are allowed for this module.',
          });
        },
      };
    },
  };
})();

exports.rules = {
  'require-namespace-imports': requireNamespaceImports,
};

// .eslintrc.js
module.exports = {
  plugins: ['local'],
  rules: {
    'local/require-namespace-imports': [
      2,
      [
        'shared/facades/option',
        'shared/facades/reactive-extensions',
        'shared/helpers/urls/search',
        'shared/helpers/urls/query',
      ],
    ]
  }
};

Questions

• Should it be possible to specify the name of the namespace to be used for each import?

[ljharb]

This seems like a very bad rule to me. Namespace imports are only for two reasons: metaprogramming and laziness. They’re the entire reason “treeshaking” exists as a concept - because namespace imports encourage files that export many things, which encourages non-deep imports, which causes too much code to end up in bundles.

Named imports can be renamed; there’s no reason i can see that every “get” needs to be the same one in the codebase.

[OliverJAsh]

namespace imports encourage files that export many things, which encourages non-deep imports, which causes too much code to end up in bundles.

I'm not sure I follow this point. Would you be able to elaborate?

(In our experience so far, namespace imports haven't affected tree shaking at all.)

[ljharb]

Specifically, if you have a file that exports 20 things, and you do import { three } from './twenty' or import * as moreThanWeNeed from './twenty', a bundler will include by default all 20 things. Treeshaking is required to try to guess which of the other 19 it can delete/omit.

If, instead, you have 20 files, each that export one thing, then no guessing is required and no omitting/deleting is required; you will have exactly the code you need in your bundle and no more.

[OliverJAsh]

Thanks for elaborating. That makes sense, but what about libraries where we don't get to decide how modules are organised?

Perhaps a real world example would help demonstrate why namespace imports are sometimes a sensible option:

import * as rxjs from 'rxjs';
import * as Rx from 'rxjs/operators';
import * as Option from 'fp-ts/lib/Option';
import * as Array from 'fp-ts/lib/Array';

const numberOptionList$: rxjs.Observable<Array<Option.Option<number>>> = rxjs.of([Option.some(1)])

const add1 = (n: number) => n + 1;

const result$ = numberOptionList$.pipe(Rx.map(Array.map(Option.map(add1))));

Each of these imports contribute named exports which have the same names, such as map, filter, and reduce (and many more).

We could use named exports with renaming:

import { map as rxMap, filter as rxFilter, reduce as rxReduce  } from 'rxjs/operators';
import { map as optionMap, filter as optionFilter, reduce as optionReduce } from 'fp-ts/lib/Option';
import { map as arrayMap, filter as arrayFilter, reduce as arrayReduce } from 'fp-ts/lib/Array';

… but that would quickly get out of hand in larger examples, where we potentially need to import even more named imports with conflicting names.

I don't think this is an edge case, or at least it won't be for much longer—as I mentioned above, there are many more JS libraries which are going in the direction of super ambiguous exports.

[ljharb]

If you’re importing so many things with conceptually colliding names in the same file that it “gets out of hand”, it seems to me like the real problem that needs fixing is that you have a module that’s doing too much and needs to be factored out into smaller pieces :-)

[OliverJAsh]

In many cases, we're composing multiple types (Option/Observable/Array, such as above), but the code which operates on these types is only a few lines long, so I don't think this problem can be solved by separating the code into smaller modules. E.g., it wouldn't make sense to separate the code in my earlier example.

[ljharb]

I guess I’m not clear on the point of this as a rule, though, even if in these sorts of files, you want to enforce that convention. It seems like this would force churn and boilerplate on the common cases for the benefit of the rare cases, where the latter can be addressed with code review.

[OliverJAsh]

These might be rare cases, but the reason we would like to enforce consistent usage is to reduce the amount of refactoring that's necessary when a module needs to start using more imports.

E.g. given a module which previously only used Option but now also needs to use Observable, if that module was previously using named imports (rather than namespace imports), it will have to be converted to namespace imports before Observable can be added.

[bbarry]

I have a use case for this in my project:

We generate a list of types and another list of redux state classes that are pretty similar to each other. The first represent the rest api requests and responses generated from an open api spec and the redux classes enable us to abstract away the api calls behind a store facade.

For example there may be an Order class which gets PUT to a /Orders endpoint. We'll have a PutOrder action class which gets dispatched and a reducer that reduces it into the application state and an effect that calls the endpoint...

Where a dev working on a component comes in the'll ideally have a pair of import statements for working with the api:

import * as Api from 'src/api';
import * as Actions from 'src/state/actions';

Requiring that within our codebase, all imports of these two barrels are made exactly this way ensures that the code using it is easier to review. We don't have to be concerned about tree shaking because all of the members of these generated files are used in the application shortly after they get added to the api itself.

To be clear I don't care for requiring namespace imports of external libraries, only ones within the application.

Ideally we could put a comment or something inside these barrels like

// @eslint-plugin-import: import-star Api

in api.ts

[RomainFallet]

I have exactly the same use case when using DDD principles and module boundaries. The use of extra prefixes is problametic as it can turn 10 lines import block into a 50 lines one.

I don't understand the struggling here, it's only a code style option. We already have the ability to disallow usage of namepsace imports, why can't we disallow usage of object imports ?

It's certainly not something that will be used on an entire codebase, but having the ability to enforce consistency on some part of the codebase is always welcome to me.