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.

Sign up for GitHub

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

Jump to bottom

[Fix] order: restore default behaviour unless type is in groups #2087

Merged
merged 1 commit into from May 20, 2021
+182 −37
Merged

[Fix] order: restore default behaviour unless type is in groups #2087

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -10,6 +10,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
- [`no-restricted-paths`]: fix false positive matches ([#2090], thanks [@malykhinvi])
- [`no-cycle`]: ignore imports where imported file only imports types of importing file ([#2083], thanks [@cherryblossom000])
- [`no-cycle`]: fix false negative when file imports a type after importing a value in Flow ([#2083], thanks [@cherryblossom000])
- [`order`]: restore default behavior unless `type` is in groups ([#2087], thanks [@grit96])

### Changed
- [Docs] Add `no-relative-packages` to list of to the list of rules ([#2075], thanks [@arvigeus])
Expand Down Expand Up @@ -792,6 +793,7 @@ for info on changes for earlier releases.
[`memo-parser`]: ./memo-parser/README.md

[#2090]: https://github.com/benmosher/eslint-plugin-import/pull/2090
[#2087]: https://github.com/benmosher/eslint-plugin-import/pull/2087
[#2083]: https://github.com/benmosher/eslint-plugin-import/pull/2083
[#2075]: https://github.com/benmosher/eslint-plugin-import/pull/2075
[#2071]: https://github.com/benmosher/eslint-plugin-import/pull/2071
Expand Down
10 changes: 7 additions & 3 deletions src/rules/order.js
View file
Open in desktop
Expand Up @@ -313,7 +313,7 @@ function computeRank(context, ranks, importEntry, excludedImportTypes) {
let rank;
if (importEntry.type === 'import:object') {
impType = 'object';
} else if (importEntry.node.importKind === 'type') {
} else if (importEntry.node.importKind === 'type' && ranks.omittedTypes.indexOf('type') === -1) {
impType = 'type';
} else {
impType = importType(importEntry.value, context);
Expand Down Expand Up @@ -382,10 +382,12 @@ function convertGroupsToRanks(groups) {
return rankObject[type] === undefined;
});

return omittedTypes.reduce(function(res, type) {
const ranks = omittedTypes.reduce(function(res, type) {
res[type] = groups.length;
return res;
}, rankObject);

return { groups: ranks, omittedTypes };
}

function convertPathGroupsForRanks(pathGroups) {
Expand Down Expand Up @@ -590,8 +592,10 @@ module.exports = {

try {
const { pathGroups, maxPosition } = convertPathGroupsForRanks(options.pathGroups || []);
const { groups, omittedTypes } = convertGroupsToRanks(options.groups || defaultGroups);
ranks = {
groups: convertGroupsToRanks(options.groups || defaultGroups),
groups,
omittedTypes,
pathGroups,
maxPosition,
};
Expand Down
207 changes: 173 additions & 34 deletions tests/src/rules/order.js
View file
Open in desktop
Expand Up @@ -2243,6 +2243,50 @@ context('TypeScript', function () {
// #1667: typescript type import support

// Option alphabetize: {order: 'asc'}
test(
{
code: `
import c from 'Bar';
import type { C } from 'Bar';
import b from 'bar';
import a from 'foo';
import type { A } from 'foo';

import index from './';
`,
parser,
options: [
{
groups: ['external', 'index'],
alphabetize: { order: 'asc' },
},
],
},
parserConfig,
),
// Option alphabetize: {order: 'desc'}
test(
{
code: `
import a from 'foo';
import type { A } from 'foo';
import b from 'bar';
import c from 'Bar';
import type { C } from 'Bar';

import index from './';
`,
parser,
options: [
{
groups: ['external', 'index'],
alphabetize: { order: 'desc' },
},
],
},
parserConfig,
),
// Option alphabetize: {order: 'asc'} with type group
test(
{
code: `
Expand All @@ -2258,14 +2302,14 @@ context('TypeScript', function () {
parser,
options: [
{
groups: ['external', 'index'],
groups: ['external', 'index', 'type'],
ljharb marked this conversation as resolved.
Show resolved Hide resolved
alphabetize: { order: 'asc' },
},
],
},
parserConfig,
),
// Option alphabetize: {order: 'desc'}
// Option alphabetize: {order: 'desc'} with type group
test(
{
code: `
Expand All @@ -2281,7 +2325,7 @@ context('TypeScript', function () {
parser,
options: [
{
groups: ['external', 'index'],
groups: ['external', 'index', 'type'],
ljharb marked this conversation as resolved.
Show resolved Hide resolved
alphabetize: { order: 'desc' },
},
],
Expand All @@ -2303,35 +2347,130 @@ context('TypeScript', function () {
},
parserConfig,
),
test(
{
code: `
import { serialize, parse, mapFieldErrors } from '@vtaits/form-schema';
import type { GetFieldSchema } from '@vtaits/form-schema';
import { useMemo, useCallback } from 'react';
import type { ReactElement, ReactNode } from 'react';
import { Form } from 'react-final-form';
import type { FormProps as FinalFormProps } from 'react-final-form';
`,
parser,
options: [
{
alphabetize: { order: 'asc' },
},
],
},
parserConfig,
),
],
invalid: [
// Option alphabetize: {order: 'asc'}
test(
{
code: `
import b from 'bar';
import c from 'Bar';
import a from 'foo';

import index from './';
import b from 'bar';
import c from 'Bar';
import type { C } from 'Bar';
import a from 'foo';
import type { A } from 'foo';

import type { A } from 'foo';
import type { C } from 'Bar';
`,
import index from './';
`,
output: `
import c from 'Bar';
import b from 'bar';
import a from 'foo';
import c from 'Bar';
import type { C } from 'Bar';
import b from 'bar';
import a from 'foo';
import type { A } from 'foo';

import index from './';
import index from './';
`,
parser,
options: [
{
groups: ['external', 'index'],
alphabetize: { order: 'asc' },
},
],
errors: [
{
message: semver.satisfies(eslintPkg.version, '< 3')
? '`bar` import should occur after import of `Bar`'
: /(`bar` import should occur after import of `Bar`)|(`Bar` import should occur before import of `bar`)/,
},
],
},
parserConfig,
),
// Option alphabetize: {order: 'desc'}
test(
{
code: `
import a from 'foo';
import type { A } from 'foo';
import c from 'Bar';
import type { C } from 'Bar';
import b from 'bar';

import type { C } from 'Bar';
import type { A } from 'foo';
`,
import index from './';
`,
output: `
import a from 'foo';
import type { A } from 'foo';
import b from 'bar';
import c from 'Bar';
import type { C } from 'Bar';

import index from './';
`,
parser,
options: [
{
groups: ['external', 'index'],
alphabetize: { order: 'desc' },
},
],
errors: [
{
message: semver.satisfies(eslintPkg.version, '< 3')
? '`bar` import should occur before import of `Bar`'
: /(`bar` import should occur before import of `Bar`)|(`Bar` import should occur after import of `bar`)/,
ljharb marked this conversation as resolved.
Show resolved Hide resolved
},
],
},
parserConfig,
),
// Option alphabetize: {order: 'asc'} with type group
test(
{
code: `
import b from 'bar';
import c from 'Bar';
import a from 'foo';

import index from './';

import type { A } from 'foo';
import type { C } from 'Bar';
`,
output: `
import c from 'Bar';
import b from 'bar';
import a from 'foo';

import index from './';

import type { C } from 'Bar';
import type { A } from 'foo';
`,
parser,
options: [
{
groups: ['external', 'index', 'type'],
alphabetize: { order: 'asc' },
},
],
Expand All @@ -2345,33 +2484,33 @@ context('TypeScript', function () {
},
parserConfig,
),
// Option alphabetize: {order: 'desc'}
// Option alphabetize: {order: 'desc'} with type group
test(
{
code: `
import a from 'foo';
import c from 'Bar';
import b from 'bar';
import a from 'foo';
import c from 'Bar';
import b from 'bar';

import index from './';
import index from './';

import type { C } from 'Bar';
import type { A } from 'foo';
`,
import type { C } from 'Bar';
import type { A } from 'foo';
`,
output: `
import a from 'foo';
import b from 'bar';
import c from 'Bar';
import a from 'foo';
import b from 'bar';
import c from 'Bar';

import index from './';
import index from './';

import type { A } from 'foo';
import type { C } from 'Bar';
`,
import type { A } from 'foo';
import type { C } from 'Bar';
`,
parser,
options: [
{
groups: ['external', 'index'],
groups: ['external', 'index', 'type'],
alphabetize: { order: 'desc' },
},
],
Expand Down