Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

fix: import-name options bugs and documentation #882

Closed
wants to merge 3 commits into from
Closed

fix: import-name options bugs and documentation #882

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
090be13
Fix `import-name` options indexing off-by-one
dosentmatter Jul 19, 2019
b26592e
Update `import-name` README.md
dosentmatter Jul 19, 2019
5e29832
Make `import-name` camelCase begin with lowercase
dosentmatter Jul 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 7 additions & 2 deletions README.md
View file
Open in desktop
Expand Up @@ -184,11 +184,16 @@ We recommend you specify exact versions of lint libraries, including `tslint-mic
</td>
<td>
The name of the imported module must match the name of the thing being imported. For special characters (<code>-</code>, <code>.</code>, <code>_</code>) remove them and make the following character uppercase.
For example, it is valid to name imported modules the same as the module name: <code>import Service = require('x/y/z/Service')</code> and <code>import Service from 'x/y/z/Service'</code>.
For example, it is valid to name imported modules the same as the module name: <code>import Service = require('./x/y/z/Service')</code> and <code>import Service from './x/y/z/Service'</code>.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this change because of defaults?

But it is invalid to change the name being imported, such as: <code>import MyCoolService = require('x/y/z/Service')</code> and <code>import MyCoolService from 'x/y/z/Service'</code>.
When containing special characters such as <code>import $$ from 'my-awesome_library';</code> the corresponding configuration would look like <code>'import-name': [true, { 'myAwesomeLibrary': '$$' }]</code>.
Since version 2.0.9 it is possible to configure this rule with a list of exceptions.
For example, to allow <code>underscore</code> to be imported as <code>_</code>, add this configuration: <code>'import-name': [ true, { 'underscore': '_' }]</code>
The default is to ignore this rule for external modules <code>ignoreExternalModule: true</code> and to use camelCase <code>case: 'camelCase'</code>.
To not ignore this rule for external modules, add this configuration: <code>'import-name': [true, null, null, { ignoreExternalModule: false }]</code>.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
To not ignore this rule for external modules, add this configuration: <code>'import-name': [true, null, null, { ignoreExternalModule: false }]</code>.
To apply this rule for external modules, add this configuration: <code>'import-name': [true, null, null, { ignoreExternalModule: false }]</code>.

To allow <code>underscore</code> to be imported as <code>_</code>, add this configuration: <code>'import-name': [true, { 'underscore': '_' }, null, { ignoreExternalModule: false }]</code>.
To ignore this rule for <code>react</code> and <code>express</code>, add this configuration: <code>'import-name': [true, null, ['react', 'express'], { ignoreExternalModule: false }]</code>.
To use PascalCase: <code>'import-name': [true, null, null, { case: 'PascalCase' }]</code>.
To use camelCase or PascalCase: <code>'import-name': [true, null, null, { case: 'any-case' }]</code>.
</td>
<td>2.0.5</td>
</tr>
Expand Down
41 changes: 11 additions & 30 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 6 additions & 4 deletions src/importNameRule.ts
View file
Open in desktop
Expand Up @@ -45,15 +45,15 @@ export class Rule extends Lint.Rules.AbstractRule {

if (options.ruleArguments instanceof Array) {
options.ruleArguments.forEach((opt: unknown, index: number) => {
if (index === 1 && isObject(opt)) {
if (index === 0 && isObject(opt)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This will be a breaking change. I thought that this might be side effect of migration to new rule format, however this behavior was in 6.0.0.
Looks like better solution is needed to avoid breaking change for those who added one more element to options array to match expected indices

result.replacements = this.extractReplacements(opt);
}

if (index === 2 && Array.isArray(opt)) {
if (index === 1 && Array.isArray(opt)) {
result.ignoredList = this.extractIgnoredList(opt);
}

if (index === 3 && isObject(opt)) {
if (index === 2 && isObject(opt)) {
result.config = this.extractConfig(opt);
}
});
Expand Down Expand Up @@ -233,12 +233,14 @@ function walk(ctx: Lint.WalkContext<Option>) {
}

function makeCamelCase(input: string): string {
return input.replace(
let result = `${input.charAt(0)}${input.slice(1)}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't look like this changes anything (without .toLowerCase())

result = result.replace(
/[-|\.|_](.)/g, // tslint:disable-next-line:variable-name
(_match: string, group1: string): string => {
return group1.toUpperCase();
}
);
return result;
}

function makePascalCase(input: string): string {
Expand Down
12 changes: 2 additions & 10 deletions src/tests/ImportNameRuleTests.ts
View file
Open in desktop
Expand Up @@ -114,7 +114,6 @@ describe('importNameRule', (): void => {
`;

const options = [
true,
{
backbone: 'Backbone',
react: 'React',
Expand All @@ -132,7 +131,6 @@ describe('importNameRule', (): void => {
import pqr from 'my-module'
`;
const options = [
true,
{
'fs/package-name': 'pkg',
'abc-tag': 'abc',
Expand All @@ -151,7 +149,6 @@ describe('importNameRule', (): void => {
import Up from 'up-module'
`;
const options = [
true,
{
'fs/package-name': 'pkg',
'abc-tag': 'abc',
Expand All @@ -171,7 +168,6 @@ describe('importNameRule', (): void => {
import Up from 'up-module'
`;
const options = [
true,
{
'fs/package-name': 'pkg',
'abc-tag': 'abc',
Expand Down Expand Up @@ -251,7 +247,6 @@ describe('importNameRule', (): void => {
`;

const options = [
true,
{},
{},
{
Expand All @@ -268,7 +263,6 @@ describe('importNameRule', (): void => {
`;

const options = [
true,
{},
{},
{
Expand Down Expand Up @@ -301,7 +295,6 @@ describe('importNameRule', (): void => {
`;

const options = [
true,
{},
{},
{
Expand All @@ -318,7 +311,6 @@ describe('importNameRule', (): void => {
`;

const options = [
true,
{},
{},
{
Expand Down Expand Up @@ -351,15 +343,15 @@ describe('importNameRule', (): void => {
import GraphqlTag from 'x/y/z/graphql-tag';
import graphqlTag from 'x/y/z/graphql-tag';
`;
const options = [true, {}, {}, { case: 'any-case' }];
const options = [{}, {}, { case: 'any-case' }];
TestHelper.assertViolationsWithOptions(ruleName, options, script, []);
});

it('should fail', () => {
const script: string = `
import gTag from 'x/y/z/graphql-tag';
`;
const options = [true, {}, {}, { case: 'any-case' }];
const options = [{}, {}, { case: 'any-case' }];
TestHelper.assertViolationsWithOptions(ruleName, options, script, [
{
failure: "Misnamed import. Import should be named 'graphqlTag' or 'GraphqlTag' but found 'gTag'",
Expand Down