Skip to content

[no-namespace] allow namespace imports for specific modules #1903

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
chimericdream opened this issue Sep 18, 2020 · 6 comments · Fixed by #2112
Closed

[no-namespace] allow namespace imports for specific modules #1903

chimericdream opened this issue Sep 18, 2020 · 6 comments · Fixed by #2112

Comments

@chimericdream
Copy link

I want to disallow using namespaces as a rule, but some third-party libraries don't use default exports. The specific one that we use quite a bit in our application is yup. Even if we use TypeScript's allowSyntheticDefaultImports rule, we still have to import the library using import * as yup from 'yup';.

I'd like to be able to do something like the following:

rules: {
    // ...
    'import/no-namespace': ['warn', {allow: ['yup', 'some-lib', '@scoped/lib']}],
    // ...
},

With the above config, the following would be considered correct:

import * as yup from 'yup';
import * as validation from 'yup';
import * as lib from 'some-lib';

And the following would be considered incorrect:

import * as nope from 'not-allowed';

// even though this contains the string "some-lib", it should not be valid; I think the allow list should be exact matches only
import * as stillNope from '@scoped/some-lib';

I came across pull request #1679 from earlier this year, but it appears it was closed without being merged.

@christianscott
Copy link

Came here to leave an issue requesting the opposite – supporting both allowed & banned namespace imports would be fantastic. Probably not both at the same time though.

rules: {
    // ...
    'import/no-namespace': ['error', {banned: ['yup', 'some-lib', '@scoped/lib']}],
    // ...
},

@ljharb
Copy link
Member

ljharb commented Sep 27, 2020

@chimericdream TS's module system is broken unless you're using both esModuleInterop and synthetic imports. At that point, you never need to use import * as - you can use all of the named exports of yup individually, you don't need them to be bound up in a namespace.

In general, import * as only has two justifications: laziness, or metaprogramming. It doesn't seem like the latter is the case here.

If we were to add such an option, we'd definitely want a way to support "all forbidden, except these" and "all allowed, except these" (but ofc not at the same time).

Can either of you help me understand why you want to use import * ever?

@christianscott
Copy link

christianscott commented Sep 28, 2020

Thanks for the reply @ljharb. Asserting that we're lazy (however true in my case) is a bit antagonistic, I'm not sure why you felt the need to include that part.

We're actually not using either of those compiler options. Even if we were I'd still like to keep using namespace imports. Stylistically, I am a fan of them.

The reasons I like them aren't super important, but two of them are:

  1. It's easier to know, at a glance, where a variable comes from if it's part of a namespace. Compare join and path.join – I find the second one a bit easier to grok
  2. It's easier, at a glance, to understand how a symbol will be used if you import it using a namespace. Using a default-style import, it's not obvious whether or not a variable will be used directly. Of course that isn't always true if esModuleInteropis not turned on. Luckily that only applies to a small number of libraries

Edit: also – I would be more than happy to implement this if you're keen to take this rule in that direction.

@chimericdream
Copy link
Author

I appreciate the replies. In my team's project at work, we're actually using both of the flags you mentioned, but there are still cases where we either prefer or are required to use namespace imports. In the case of yup, for example, we can't use the members of the namespace directly, because they have names like string, number, and object. If we wanted to use them without a namespace import, we'd have to do something like the following:

import {string as yupString, number as yupNumber, object as yupObject, ...} from 'yup';

While syntactically valid, this is just overly verbose in my opinion. I also agree with the two points @christianscott mentioned regarding readability and quicker recognition. On the whole, I prefer to use namespaces as little as possible. But there are a small handful of cases in the projects I work on where avoiding them isn't all that straightforward.

@ljharb
Copy link
Member

ljharb commented Sep 28, 2020

@christianscott i didn't mean it antagonistically; what i meant was, explicit is better than implicit, and it's far better for tons of reasons to explicitly enumerate all needed named imports, syntactically.

@chimericdream that's a fair point, and for a library like that, where there's no default export, just a collection of common names, there's a reasonable argument to be made in favor of import * as.

In the path case, path is the default export - import path from 'path' is the only correct form, full stop. It's only a spec-violating babel transform that would allow you do to import { join } from 'path' in the first place. In other words, for cases like this, the proper thing to do is for the module to export a default export that is, itself, the namespace.

@christianscott I'm not sure I understand your second point - how is a default import vs a named import any different in describing whether a variable will be used directly? They're all identical identifiers; short of grepping or an eslint rule, you can't ever know if they'll be used directly - and with those things, you always know.

I'd be fine with a PR adding an option to meet this use case.

@chimericdream
Copy link
Author

I created a PR with what I have so far. Unfortunately I cannot get the tests to run on my Windows machine. @christianscott or @ljharb, would either of you be able to help me get this implementation completed so we can merge it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants