From 9032868362b419144f497d0546212c53741f7548 Mon Sep 17 00:00:00 2001 From: Rahul Gupta Date: Thu, 14 Jan 2021 21:22:56 +0530 Subject: [PATCH 1/4] Add unassignedImports option, Update Readme --- docs/rules/order-imports.md | 56 ++++++++++++++++++++++++++++++++++++- src/rules/order-imports.ts | 10 ++++++- 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/docs/rules/order-imports.md b/docs/rules/order-imports.md index e09281a..a0cdbe6 100644 --- a/docs/rules/order-imports.md +++ b/docs/rules/order-imports.md @@ -23,7 +23,7 @@ import main from './'; Notes: -- Unassigned imports are ignored (ex: `import 'polyfill'`), as the order they are imported in may be important. +- Unassigned imports are ignored (ex: `import 'polyfill'`), as the order they are imported in may be important. Use 'unassignedImports' option if you'd like to allow them. - Statements using the ES6 `import` syntax must appear before any `require()` statements. ## Usage @@ -149,6 +149,60 @@ import bar from 'bar'; import Baz from 'Baz'; ``` +### `unassignedImports: [ignore|allow] (default: ignore)` + +Unassigned imports refers to imports which are not assigned to any variable but are imported globally. + +Example: +```js +import 'polyfill' +import 'containers/styles.scss' +``` + +By default unassigned imports are ignored, as the order they are imported in may be important. + +- If set to `allow`, considers unassigned imports like any other imports when ordering. +- If set to `ignore`, does not consider the ordering for this import. + +Examples: + +#### ignore +```js +/* eslint import-helpers/order-imports: [ + "error", + { + unassignedImports: 'ignore', + groups: [['module'], '/\.scss$/'] + }, +] */ + +/* Any placement of 'styles.scss' is VALID */ +import 'styles.scss'; +import fs from 'fs'; +import path from 'path'; +``` + +#### allow +```js +/* eslint import-helpers/order-imports: [ + "error", + { + unassignedImports: 'allow', + groups: [['module'], '/\.scss$/'] + }, +] */ + +/* INVALID */ +import 'styles.scss' +import fs from 'fs'; +import path from 'path'; + +/* VALID */ +import fs from 'fs'; +import path from 'path'; +import 'styles.scss' +``` + ## Upgrading from v0.14 to v1 ### `builtin` | `external` | `internal` → `module` diff --git a/src/rules/order-imports.ts b/src/rules/order-imports.ts index c6c2d0e..d9f2a52 100644 --- a/src/rules/order-imports.ts +++ b/src/rules/order-imports.ts @@ -17,10 +17,14 @@ const alphabetizeOptions: AlphabetizeOption[] = ['ignore', 'asc', 'desc']; type Groups = (ValidImportType | ValidImportType[])[]; const defaultGroups: Groups = ['absolute', 'module', 'parent', 'sibling', 'index']; +type UnassignedImportsOption = 'allow' | 'ignore'; +const unassignedImportsOption: UnassignedImportsOption[] = ['allow', 'ignore']; + type RuleOptions = { groups?: Groups; newlinesBetween?: NewLinesBetweenOption; alphabetize?: Partial; + unassignedImports?: UnassignedImportsOption; }; type ImportType = 'require' | 'import'; @@ -496,6 +500,9 @@ module.exports = { newlinesBetween: { enum: newLinesBetweenOptions, }, + unassignedImports: { + enum: unassignedImportsOption, + }, alphabetize: { type: 'object', properties: { @@ -518,6 +525,7 @@ module.exports = { create: function importOrderRule(context) { const options: RuleOptions = context.options[0] || {}; const newlinesBetweenImports: NewLinesBetweenOption = options.newlinesBetween || 'ignore'; + const unassignedImports = options.unassignedImports || 'ignore'; let alphabetize: AlphabetizeConfig; let ranks: Ranks; @@ -543,7 +551,7 @@ module.exports = { return { ImportDeclaration: function handleImports(node) { - if (node.specifiers.length) { + if (node.specifiers.length || unassignedImports === 'allow') { // Ignoring unassigned imports const name: string = node.source.value; registerNode(node, name, 'import', ranks, regExpGroups, imported); From 48a8f20c1a02b0f1d5557ab1645e6707e43656c3 Mon Sep 17 00:00:00 2001 From: Rahul Gupta Date: Thu, 14 Jan 2021 21:53:14 +0530 Subject: [PATCH 2/4] Add test for unassignedImports option --- docs/rules/order-imports.md | 2 +- test/rules/order-imports.js | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/docs/rules/order-imports.md b/docs/rules/order-imports.md index a0cdbe6..54268a5 100644 --- a/docs/rules/order-imports.md +++ b/docs/rules/order-imports.md @@ -156,7 +156,7 @@ Unassigned imports refers to imports which are not assigned to any variable but Example: ```js import 'polyfill' -import 'containers/styles.scss' +import 'styles.scss' ``` By default unassigned imports are ignored, as the order they are imported in may be important. diff --git a/test/rules/order-imports.js b/test/rules/order-imports.js index 5fbd7fc..42d1109 100644 --- a/test/rules/order-imports.js +++ b/test/rules/order-imports.js @@ -1471,5 +1471,31 @@ comment3 */", // the spacing here is really sensitive }, ], }), + // Option unassignedImports: 'allow' should consider unassigned imports when sorting for respective groups + test({ + code: ` + import './an-unassigned-relative'; + import path from 'path'; + import _ from './relative'; + import 'an-unassigned-module'; + `, + output: ` + import './an-unassigned-relative'; + import path from 'path'; + import _ from './relative'; + import 'an-unassigned-module'; + `, + options: [{ unassignedImports: 'allow' }], + errors: [ + { + line: 3, + message: '`path` import should occur before import of `./an-unassigned-relative`', + }, + { + line: 5, + message: '`an-unassigned-module` import should occur before import of `./an-unassigned-relative`', + }, + ], + }), ], }); From 97f43e3a2b7c4071c83be99ffa3e624afe36cc56 Mon Sep 17 00:00:00 2001 From: Rahul Gupta Date: Fri, 29 Jan 2021 11:54:18 +0530 Subject: [PATCH 3/4] Handle fixer for Imports & Update test --- src/rules/order-imports.ts | 28 ++++++++++++++++++---------- test/rules/order-imports.js | 33 ++++++++++++++++++--------------- 2 files changed, 36 insertions(+), 25 deletions(-) diff --git a/src/rules/order-imports.ts b/src/rules/order-imports.ts index d9f2a52..ec1960b 100644 --- a/src/rules/order-imports.ts +++ b/src/rules/order-imports.ts @@ -182,21 +182,24 @@ function isPlainRequireModule(node): boolean { ); } -function isPlainImportModule(node: NodeOrToken): boolean { - return node.type === 'ImportDeclaration' && node.specifiers != null && node.specifiers.length > 0; +function isAllowedImportModule(node: NodeOrToken, context): boolean { + const unassignedImportsAllowed = getOptions(context).unassignedImports === 'allow'; + const hasNodeSpecifier = node.specifiers != null && node.specifiers.length > 0; + + return node.type === 'ImportDeclaration' && (hasNodeSpecifier || unassignedImportsAllowed); } -function canCrossNodeWhileReorder(node: NodeOrToken): boolean { - return isPlainRequireModule(node) || isPlainImportModule(node); +function canCrossNodeWhileReorder(node: NodeOrToken, context): boolean { + return isPlainRequireModule(node) || isAllowedImportModule(node, context); } -function canReorderItems(firstNode: NodeOrToken, secondNode: NodeOrToken): boolean { +function canReorderItems(firstNode: NodeOrToken, secondNode: NodeOrToken, context): boolean { const parent = firstNode.parent; const firstIndex = parent.body.indexOf(firstNode); const secondIndex = parent.body.indexOf(secondNode); const nodesBetween = parent.body.slice(firstIndex, secondIndex + 1); for (var nodeBetween of nodesBetween) { - if (!canCrossNodeWhileReorder(nodeBetween)) { + if (!canCrossNodeWhileReorder(nodeBetween, context)) { return false; } } @@ -213,7 +216,7 @@ function fixOutOfOrder(context, firstNode: NodeOrToken, secondNode: NodeOrToken, const secondRoot = findRootNode(secondNode.node); const secondRootStart = findStartOfLineWithComments(sourceCode, secondRoot); const secondRootEnd = findEndOfLineWithComments(sourceCode, secondRoot); - const canFix = canReorderItems(firstRoot, secondRoot); + const canFix = canReorderItems(firstRoot, secondRoot, context); let newCode = sourceCode.text.substring(secondRootStart, secondRootEnd); if (newCode[newCode.length - 1] !== '\n') { @@ -482,6 +485,12 @@ function getAlphabetizeConfig(options: RuleOptions): AlphabetizeConfig { return { order, ignoreCase }; } +function getOptions(context) { + const options: RuleOptions = context.options[0] || {}; + + return options; +} + module.exports = { meta: { type: 'suggestion', @@ -523,9 +532,8 @@ module.exports = { }, create: function importOrderRule(context) { - const options: RuleOptions = context.options[0] || {}; + const options = getOptions(context); const newlinesBetweenImports: NewLinesBetweenOption = options.newlinesBetween || 'ignore'; - const unassignedImports = options.unassignedImports || 'ignore'; let alphabetize: AlphabetizeConfig; let ranks: Ranks; @@ -551,7 +559,7 @@ module.exports = { return { ImportDeclaration: function handleImports(node) { - if (node.specifiers.length || unassignedImports === 'allow') { + if (isAllowedImportModule(node, context)) { // Ignoring unassigned imports const name: string = node.source.value; registerNode(node, name, 'import', ranks, regExpGroups, imported); diff --git a/test/rules/order-imports.js b/test/rules/order-imports.js index 42d1109..6c8dcf2 100644 --- a/test/rules/order-imports.js +++ b/test/rules/order-imports.js @@ -108,6 +108,15 @@ ruleTester.run('order', rule, { import path from 'path'; `, }), + // Consider unassigned values when option is provided (import) + test({ + code: ` + import 'fs'; + import path from 'path'; + import './foo'; + `, + options: [{ unassignedImports: 'allow' }], + },), // No imports test({ code: ` @@ -1471,29 +1480,23 @@ comment3 */", // the spacing here is really sensitive }, ], }), - // Option unassignedImports: 'allow' should consider unassigned imports when sorting for respective groups + // Option unassignedImports: 'allow' should consider unassigned module imports test({ code: ` - import './an-unassigned-relative'; - import path from 'path'; - import _ from './relative'; - import 'an-unassigned-module'; + import './foo'; + import 'fs'; + import path from 'path'; `, output: ` - import './an-unassigned-relative'; - import path from 'path'; - import _ from './relative'; - import 'an-unassigned-module'; + import 'fs'; + import path from 'path'; + import './foo'; `, options: [{ unassignedImports: 'allow' }], errors: [ { - line: 3, - message: '`path` import should occur before import of `./an-unassigned-relative`', - }, - { - line: 5, - message: '`an-unassigned-module` import should occur before import of `./an-unassigned-relative`', + line: 2, + message: '`./foo` import should occur after import of `path`', }, ], }), From 6d92f7a57befd977e532026f9c43a4430dd60209 Mon Sep 17 00:00:00 2001 From: Rahul Gupta Date: Tue, 23 Mar 2021 01:39:32 +0530 Subject: [PATCH 4/4] Handle unassignedImports for require statement --- src/rules/order-imports.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/rules/order-imports.ts b/src/rules/order-imports.ts index ec1960b..0cc2edc 100644 --- a/src/rules/order-imports.ts +++ b/src/rules/order-imports.ts @@ -182,11 +182,12 @@ function isPlainRequireModule(node): boolean { ); } +const isUnassignedImportsAllowed = (context) => getOptions(context).unassignedImports === 'allow'; + function isAllowedImportModule(node: NodeOrToken, context): boolean { - const unassignedImportsAllowed = getOptions(context).unassignedImports === 'allow'; const hasNodeSpecifier = node.specifiers != null && node.specifiers.length > 0; - return node.type === 'ImportDeclaration' && (hasNodeSpecifier || unassignedImportsAllowed); + return node.type === 'ImportDeclaration' && (hasNodeSpecifier || isUnassignedImportsAllowed(context)); } function canCrossNodeWhileReorder(node: NodeOrToken, context): boolean { @@ -566,7 +567,8 @@ module.exports = { } }, CallExpression: function handleRequires(node) { - if (level !== 0 || !isStaticRequire(node) || !isInVariableDeclarator(node.parent)) { + const isUnassignedRequire = !isInVariableDeclarator(node.parent); + if (level !== 0 || !isStaticRequire(node) || (!isUnassignedImportsAllowed(context) && isUnassignedRequire)) { return; } const name: string = node.arguments[0].value;