-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Add type import support to import/order #645
Comments
+1. I import my types with
|
I think this makes sense, need @jfmengels to manage the resolution, though. Not sure if this is a new |
We could make that a new if some people want to have their imports glued together without empty lines, then the type imports, with both groups separated by a line. import path from 'path';
import a from './a';
import type Foo from 'foo'; Or if some people want to have their types next to the imported package import path from 'path';
import foo from 'foo';
import type Foo from 'foo';
import a from './a'; That said, you'd get the same problem if you wanted to order things by resource type (js, json, css, types, ...), which is not possible at the moment. Also, maybe you'd want to order your type imports by the import type of the module. import type A from './a'; // this import should be after import of 'foo'
import type Foo from 'foo'; Any thoughts? |
yeah, that's where I almost feel like some sort of tiered ordering is needed, but I don't know how it would be specified. the existing types are a function of the import path, but this new which is how you end up with the options you've described. |
maybe just ignore type imports. I personally prefer,
as types are just compile time thing, and get a good sense of all the imports if I group them. If this plugin ignores type imports, then user can either separate it at the end as below, or together with the module import. |
In Flow v0.38.0, there is a new feature that allows you to import both values and types in a single import statement.
I haven't used it yet, but that sounds better and I feel like we should encourage the use of that syntax rather than one separating the import of types and the import of values. Maybe eslint-plugin-flow could implement a rule enforcing the use of that syntax @gajus ? What do you think? |
A problem I frequently run into, as well, is importing types from relative vs absolute paths. // @flow
import fs from 'fs';
import Foo from './FooClass';
import type { SomeGlobalType } from 'my-global-types-pkg';
import type { LocalModuleType } from './types'; It makes sense to me to group the types together as it makes reading the code more natural. Probably related with https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/first.md as well. Any thoughts? |
@jfmengels , but what if i have to import the default module and a type? /* @flow */
import type { awesomeType } from './awesomeModule.js'
import awesomeDefaultModule from './awesomeModule.js' |
@emil14 you can do: import awesomeDefaultModule, { type awesomeType } from './awesomeModule.js' |
@christophehurpeau wow thank you, I'll try it |
Any news on this? I also feel that @jribeiro's suggestion feels the most natural. I'd prefer not mix type imports with other stuff. Regardless, if the gods of js say otherwise, I'd adapt. |
Also interested in this rule. |
This is coming to TypeScript in 3.8 as well: https://devblogs.microsoft.com/typescript/announcing-typescript-3-8-rc/#type-only-imports-exports |
I've written a patch that adds type import support and handles alphabetisation when using separate type imports for the same module. Example importsimport storage from '@react-native-async-storage/async-storage';
import {
offlineActionTypes,
checkInternetConnection
} from 'react-native-offline';
import { composeWithDevTools } from 'redux-devtools-extension';
import { createMigrate, persistStore, persistReducer } from 'redux-persist';
import thunk from 'redux-thunk';
import { createStore, applyMiddleware } from 'redux';
import { migrations, version, key } from 'app/store/migrations';
import rootReducer from 'app/store/reducers';
import APIConnector from 'app/utils/api-connector';
import { loadDeviceInfo } from 'app/utils/device-info';
import type { StateType } from 'app/store/reducers';
import type { Persistor } from 'redux-persist/src/types';
import type { Store, DispatchAPI } from 'redux'; .eslintrc{
"rules": {
"import/order": [
"error",
{
"groups": ["external", "internal", "types"],
"pathGroups": [
{
"pattern": "app/**",
"group": "internal"
}
],
"pathGroupsExcludedImportTypes": ["types"],
"newlines-between": "always",
"alphabetize": { "order": "asc" }
}
]
}
} eslint-plugin-import+2.22.1.patchdiff --git a/node_modules/eslint-plugin-import/lib/rules/order.js b/node_modules/eslint-plugin-import/lib/rules/order.js
index d3cd442..733af22 100644
--- a/node_modules/eslint-plugin-import/lib/rules/order.js
+++ b/node_modules/eslint-plugin-import/lib/rules/order.js
@@ -265,7 +265,7 @@ function mutateRanksToAlphabetize(imported, alphabetizeOptions) {
if (!Array.isArray(acc[importedItem.rank])) {
acc[importedItem.rank] = [];
}
- acc[importedItem.rank].push(importedItem.value);
+ acc[importedItem.rank].push(`${importedItem.value}-${importedItem.node.importKind}`);
return acc;
}, {});
@@ -290,7 +290,7 @@ function mutateRanksToAlphabetize(imported, alphabetizeOptions) {
// mutate the original group-rank with alphabetized-rank
imported.forEach(function (importedItem) {
- importedItem.rank = alphabetizedRanks[importedItem.value];
+ importedItem.rank = alphabetizedRanks[`${importedItem.value}-${importedItem.node.importKind}`];
});
}
@@ -310,6 +310,8 @@ function computeRank(context, ranks, importEntry, excludedImportTypes) {
let rank;
if (importEntry.type === 'import:object') {
impType = 'object';
+ } else if (importEntry.type === 'import' && importEntry.node.importKind === 'type') {
+ impType = 'types';
} else {
impType = (0, _importType2.default)(importEntry.value, context);
}
@@ -338,7 +340,7 @@ function isInVariableDeclarator(node) {
node.type === 'VariableDeclarator' || isInVariableDeclarator(node.parent));
}
-const types = ['builtin', 'external', 'internal', 'unknown', 'parent', 'sibling', 'index', 'object'];
+const types = ['builtin', 'external', 'internal', 'unknown', 'parent', 'sibling', 'index', 'object', 'types'];
// Creates an object with type-rank pairs.
// Example: { index: 0, sibling: 1, parent: 1, external: 1, builtin: 2, internal: 2 }
|
@grit96 would you mind making a PR with that patch, and some test cases? |
Is this a breaking change for TypeScript users that are importing types together with the primary import? eg import { Bar } from 'bar';
import type { BarType } from 'bar';
import { Foo } from 'foo'; I don't believe TypeScript supports importing both types and non-types in the same import line like flow allows. import { Bar, type BarType } from 'bar'; Is there a way to add an option to retain the previous behaviour? |
Yes, it seems like it might be. See #2070. |
Example:
Refer to https://flowtype.org/blog/2015/02/18/Import-Types.html
The text was updated successfully, but these errors were encountered: