Skip to content

Commit

Permalink
[Fix] order: partial fix for import-js#2687
Browse files Browse the repository at this point in the history
  • Loading branch information
ljharb committed Apr 14, 2023
1 parent a89eadf commit 1fa2971
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 10 deletions.
12 changes: 7 additions & 5 deletions src/rules/order.js
Expand Up @@ -253,6 +253,7 @@ function makeOutOfOrderReport(context, imported) {
if (!outOfOrder.length) {
return;
}

// There are things to report. Try to minimize the number of reported errors.
const reversedImported = reverse(imported);
const reversedOrder = findOutOfOrder(reversedImported);
Expand Down Expand Up @@ -426,11 +427,12 @@ const types = ['builtin', 'external', 'internal', 'unknown', 'parent', 'sibling'
// Example: { index: 0, sibling: 1, parent: 1, external: 1, builtin: 2, internal: 2 }
// Will throw an error if it contains a type that does not exist, or has a duplicate
function convertGroupsToRanks(groups) {
if (groups.length === 1) {
// TODO: remove this `if` and fix the bug
return convertGroupsToRanks(groups[0]);
}
const rankObject = groups.reduce(function (res, group, index) {
if (typeof group === 'string') {
group = [group];
}
group.forEach(function (groupItem) {
[].concat(group).forEach(function (groupItem) {
if (types.indexOf(groupItem) === -1) {
throw new Error(`Incorrect configuration of the rule: Unknown type \`${JSON.stringify(groupItem)}\``);
}
Expand All @@ -443,7 +445,7 @@ function convertGroupsToRanks(groups) {
}, {});

const omittedTypes = types.filter(function (type) {
return rankObject[type] === undefined;
return typeof rankObject[type] === 'undefined';
});

const ranks = omittedTypes.reduce(function (res, type) {
Expand Down
4 changes: 4 additions & 0 deletions tests/src/core/importType.js
Expand Up @@ -17,7 +17,11 @@ describe('importType(name)', function () {

it("should return 'builtin' for node.js modules", function () {
expect(importType('fs', context)).to.equal('builtin');
expect(importType('node:fs', context)).to.equal('builtin');
expect(importType('fs/promises', context)).to.equal('builtin');
expect(importType('node:fs/promises', context)).to.equal('builtin');
expect(importType('path', context)).to.equal('builtin');
expect(importType('node:path', context)).to.equal('builtin');
});

it("should return 'external' for non-builtin modules without a relative path", function () {
Expand Down
73 changes: 68 additions & 5 deletions tests/src/rules/order.js
Expand Up @@ -3174,18 +3174,81 @@ context('TypeScript', function () {
import type { ParsedPath } from 'path';
}
`,
errors: [{
message: '`fs` type import should occur before type import of `path`',
}, {
message: '`fs` type import should occur before type import of `path`',
}],
errors: [
{ message: '`fs` type import should occur before type import of `path`' },
{ message: '`fs` type import should occur before type import of `path`' },
],
...parserConfig,
options: [
{
alphabetize: { order: 'asc' },
},
],
}),

test({
code: `
import express from 'express';
import log4js from 'log4js';
import chpro from 'node:child_process';
// import fsp from 'node:fs/promises';
`,
output: `
import chpro from 'node:child_process';
import express from 'express';
import log4js from 'log4js';
// import fsp from 'node:fs/promises';
`,
options: [{
groups: [
'builtin',
'external',
'internal',
'parent',
'sibling',
'index',
'object',
'type',
],
}],
errors: [
{ message: '`node:child_process` import should occur before import of `express`' },
// { message: '`node:fs/promises` import should occur before import of `express`' },
],
}),

test({
code: `
import express from 'express';
import log4js from 'log4js';
import chpro from 'node:child_process';
// import fsp from 'node:fs/promises';
`,
output: `
import chpro from 'node:child_process';
import express from 'express';
import log4js from 'log4js';
// import fsp from 'node:fs/promises';
`,
options: [{
groups: [
[
'builtin',
'external',
'internal',
'parent',
'sibling',
'index',
'object',
'type',
],
],
}],
errors: [
{ message: '`node:child_process` import should occur before import of `express`' },
// { message: '`node:fs/promises` import should occur before import of `express`' },
],
}),
],
});
});
Expand Down

0 comments on commit 1fa2971

Please sign in to comment.