Skip to content
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

Added allow and forbid options to no-namespace rule #1679

Closed
wants to merge 1 commit into from
Closed

Added allow and forbid options to no-namespace rule #1679

wants to merge 1 commit into from

Conversation

el-angel
Copy link

@el-angel el-angel commented Mar 6, 2020

I've added allow and forbid options, so this rule can be used for certain namespaces, instead for all

@coveralls
Copy link

Coverage Status

Coverage increased (+0.006%) to 97.803% when pulling 86793f5 on el-angel:master into efd6be1 on benmosher:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.006%) to 97.803% when pulling 86793f5 on el-angel:master into efd6be1 on benmosher:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.006%) to 97.803% when pulling 86793f5 on el-angel:master into efd6be1 on benmosher:master.


This rule takes the following options:

`allow`: an array of namespaces the rule allows
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i assume this should cover specifiers, not the binding names?

(having the binding name and the specifier be the same in most of the examples make it unclear to which this applies)

Copy link
Author

@el-angel el-angel Mar 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the allow option works as follows:

// { allow: ['myModule', 'React'] }
import * as React from 'react';
import * as myModule from '../module/randomFile.js';

@ljharb
Copy link
Member

ljharb commented Mar 6, 2020

Also, can you elaborate on the use case here?

@el-angel
Copy link
Author

el-angel commented Mar 9, 2020

Use cases would be there you want to force certain namespaces, and forbid others. This rule would make it very easy to force consistency and allow:

// { allow: ['constants'] }
import * as constants from '../constants.js;

while it would disallow:
import * as C from '../constants.js
for example.

Also you could make it possible to only disallow:

// { forbid: ['React'] }
import * as React from 'react';

while allowing all other namespace imports

@el-angel el-angel requested a review from ljharb April 1, 2020 08:40
@ljharb
Copy link
Member

ljharb commented Jun 7, 2020

I'm not clear tho on why you'd want any path to be imported as constants?

It seems more like what you want is to point to a specific file, and allow that one to be imported as a namespace with a specific name?

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

Successfully merging this pull request may close these issues.

3 participants