-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
[New] import/order
: add support for named imports
#3043
Conversation
fb204ee
to
f1ae2d6
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3043 +/- ##
==========================================
+ Coverage 95.31% 95.46% +0.14%
==========================================
Files 82 82
Lines 3438 3549 +111
Branches 1186 1244 +58
==========================================
+ Hits 3277 3388 +111
Misses 161 161 ☔ View full report in Codecov by Sentry. |
58a89a6
to
33b5621
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a number of lines not covered by tests (see the annotations in the diff tab)
5d0b93a
to
9a43a2d
Compare
Alright! I guess I'm ready |
badcd8c
to
a2cc811
Compare
I'll take a look at whatever tests fail. |
b70cbaa
to
353a5a6
Compare
353a5a6
to
46fe08d
Compare
I had to switch to draft for an instant here, because I have noticed that the named ordering did not work if they are spreaded across multiple lines. I was able to fix this now - thus switching back to |
@manuth any reason for the custom |
The |
That's what the string.prototype.trimend package is for. |
Ooh weird - I felt like the checks once didn't run through cause of it I'll try to change it back, then! Thanks 😄 |
oh no, what i mean is, add that package as a dependency and import it instead of rolling your own |
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/eslint@2.13.1), npm/find-up@1.1.2), npm/glob@1.0.0), npm/jackspeak@=2.1.1), npm/jquery@3.7.1), npm/jsonc-parser@=3.2.0), npm/linklocal@2.8.2), npm/lodash.cond@4.5.2), npm/lodash.isarray@4.0.0), npm/markdownlint-cli@0.35.0), npm/npm-which@3.0.1), npm/object.fromentries@2.0.8), npm/object.groupby@1.0.3), npm/object.values@1.2.0), npm/react@16.14.0), npm/semver@6.3.1), npm/tsconfig-paths@3.15.0) |
🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎ To accept the risk, merge this PR and you will not be notified again.
Next stepsWhat is a critical CVE?Contains a Critical Common Vulnerability and Exposure (CVE). Remove or replace dependencies that include known critical CVEs. Consumers can use dependency overrides or npm audit fix --force to remove vulnerable dependencies. Take a deeper look at the dependencyTake a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev. Remove the packageIf you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency. Mark a package as acceptable riskTo ignore an alert, reply with a comment starting with
|
f4c2dbf
to
6f48deb
Compare
k, i've rebased this and i think it's ready to land, but i'm going to wait til i get a patch release out first. |
Awesome, thank you so much 😄 |
Just saw a little inelegance in the JSON schema I have written |
6f48deb
to
95849c8
Compare
| datasource | package | from | to | | ---------- | -------------------- | ------ | ------ | | npm | eslint-plugin-import | 2.30.0 | 2.31.0 | ## [v2.31.0](https://github.com/import-js/eslint-plugin-import/blob/HEAD/CHANGELOG.md#2310---2024-10-03) ##### Added - support eslint v9 (\[[#2996](import-js/eslint-plugin-import#2996)], thanks \[[@G-Rath](https://github.com/G-Rath)] \[[@michaelfaith](https://github.com/michaelfaith)]) - \[`order`]: allow validating named imports (\[[#3043](import-js/eslint-plugin-import#3043)], thanks \[[@manuth](https://github.com/manuth)]) - \[`extensions`]: add the `checkTypeImports` option (\[[#2817](import-js/eslint-plugin-import#2817)], thanks \[[@phryneas](https://github.com/phryneas)]) ##### Fixed - `ExportMap` / flat config: include `languageOptions` in context (\[[#3052](import-js/eslint-plugin-import#3052)], thanks \[[@michaelfaith](https://github.com/michaelfaith)]) - \[`no-named-as-default`]: Allow using an identifier if the export is both a named and a default export (\[[#3032](import-js/eslint-plugin-import#3032)], thanks \[[@akwodkiewicz](https://github.com/akwodkiewicz)]) - \[`export`]: False positive for exported overloaded functions in TS (\[[#3065](import-js/eslint-plugin-import#3065)], thanks \[[@liuxingbaoyu](https://github.com/liuxingbaoyu)]) - `exportMap`: export map cache is tainted by unreliable parse results (\[[#3062](import-js/eslint-plugin-import#3062)], thanks \[[@michaelfaith](https://github.com/michaelfaith)]) - `exportMap`: improve cacheKey when using flat config (\[[#3072](import-js/eslint-plugin-import#3072)], thanks \[[@michaelfaith](https://github.com/michaelfaith)]) - adjust "is source type module" checks for flat config (\[[#2996](import-js/eslint-plugin-import#2996)], thanks \[[@G-Rath](https://github.com/G-Rath)]) ##### Changed - \[Docs] \[`no-relative-packages`]: fix typo (\[[#3066](import-js/eslint-plugin-import#3066)], thanks \[[@joshuaobrien](https://github.com/joshuaobrien)]) - \[Performance] \[`no-cycle`]: dont scc for each linted file (\[[#3068](import-js/eslint-plugin-import#3068)], thanks \[[@soryy708](https://github.com/soryy708)]) - \[Docs] \[`no-cycle`]: add `disableScc` to docs (\[[#3070](import-js/eslint-plugin-import#3070)], thanks \[[@soryy708](https://github.com/soryy708)]) - \[Tests] use re-exported `RuleTester` (\[[#3071](import-js/eslint-plugin-import#3071)], thanks \[[@G-Rath](https://github.com/G-Rath)]) - \[Docs] \[`no-restricted-paths`]: fix grammar (\[[#3073](import-js/eslint-plugin-import#3073)], thanks \[[@unbeauvoyage](https://github.com/unbeauvoyage)]) - \[Tests] \[`no-default-export`], \[`no-named-export`]: add test case (thanks \[[@G-Rath](https://github.com/G-Rath)])
| datasource | package | from | to | | ---------- | -------------------- | ------ | ------ | | npm | eslint-plugin-import | 2.30.0 | 2.31.0 | ## [v2.31.0](https://github.com/import-js/eslint-plugin-import/blob/HEAD/CHANGELOG.md#2310---2024-10-03) ##### Added - support eslint v9 (\[[#2996](import-js/eslint-plugin-import#2996)], thanks \[[@G-Rath](https://github.com/G-Rath)] \[[@michaelfaith](https://github.com/michaelfaith)]) - \[`order`]: allow validating named imports (\[[#3043](import-js/eslint-plugin-import#3043)], thanks \[[@manuth](https://github.com/manuth)]) - \[`extensions`]: add the `checkTypeImports` option (\[[#2817](import-js/eslint-plugin-import#2817)], thanks \[[@phryneas](https://github.com/phryneas)]) ##### Fixed - `ExportMap` / flat config: include `languageOptions` in context (\[[#3052](import-js/eslint-plugin-import#3052)], thanks \[[@michaelfaith](https://github.com/michaelfaith)]) - \[`no-named-as-default`]: Allow using an identifier if the export is both a named and a default export (\[[#3032](import-js/eslint-plugin-import#3032)], thanks \[[@akwodkiewicz](https://github.com/akwodkiewicz)]) - \[`export`]: False positive for exported overloaded functions in TS (\[[#3065](import-js/eslint-plugin-import#3065)], thanks \[[@liuxingbaoyu](https://github.com/liuxingbaoyu)]) - `exportMap`: export map cache is tainted by unreliable parse results (\[[#3062](import-js/eslint-plugin-import#3062)], thanks \[[@michaelfaith](https://github.com/michaelfaith)]) - `exportMap`: improve cacheKey when using flat config (\[[#3072](import-js/eslint-plugin-import#3072)], thanks \[[@michaelfaith](https://github.com/michaelfaith)]) - adjust "is source type module" checks for flat config (\[[#2996](import-js/eslint-plugin-import#2996)], thanks \[[@G-Rath](https://github.com/G-Rath)]) ##### Changed - \[Docs] \[`no-relative-packages`]: fix typo (\[[#3066](import-js/eslint-plugin-import#3066)], thanks \[[@joshuaobrien](https://github.com/joshuaobrien)]) - \[Performance] \[`no-cycle`]: dont scc for each linted file (\[[#3068](import-js/eslint-plugin-import#3068)], thanks \[[@soryy708](https://github.com/soryy708)]) - \[Docs] \[`no-cycle`]: add `disableScc` to docs (\[[#3070](import-js/eslint-plugin-import#3070)], thanks \[[@soryy708](https://github.com/soryy708)]) - \[Tests] use re-exported `RuleTester` (\[[#3071](import-js/eslint-plugin-import#3071)], thanks \[[@G-Rath](https://github.com/G-Rath)]) - \[Docs] \[`no-restricted-paths`]: fix grammar (\[[#3073](import-js/eslint-plugin-import#3073)], thanks \[[@unbeauvoyage](https://github.com/unbeauvoyage)]) - \[Tests] \[`no-default-export`], \[`no-named-export`]: add test case (thanks \[[@G-Rath](https://github.com/G-Rath)])
import/order
: add support for named importsChanges made in this PR change the
import/order
rule to allow configuring the order of named specifiers inimport
,export
andrequire
statements.This will report errors and automatically fix expressions as shown in this example:
Reports error:
Fixed code:
This feature heavily uses the pre-existing functions used for re-ordering import/export statements and thus does not require a lot of changes.
To address the requirement brought forward by @ronparkdev, there are 3 options
named.import
,named.export
andnamed.require
for individually forcing ordering of named items for the specified category.Furthermore, in response to the comment written by @JensDll, there is an option
types
which allows to specify the order of the specifiers based on their category:mixed
: all specifiers are ordered alphabeticallytypes-first
:type
imports must occur firsttypes-last
: value imports must occur firstWith this option, one could force
type
imports to occur last by setting the options of theorder
rule to the following:Things to consider
Following tiny bit of code makes use of
const gapCode = sourceCode.text.substring(firstRootEnd, secondRootStart).replace(/,$/, '');
const gapCode = sourceCode.text.substring(secondRootEnd, firstRootStart).replace(/^,/, '');
RegExp
:eslint-plugin-import/src/rules/order.js
Line 239 in 60b0493
eslint-plugin-import/src/rules/order.js
Line 250 in 60b0493
The lines between thedefault
and thetype
/value
groups are a little blurred as adefault
re-export technically still is either avalue
or atype
export.default
will take highest priority which means that bothexport { type default as Readline } from 'foo'
andexport { default as Readline } from 'foo'
will have their group set todefault
.const { Alpha: Bravo, Alpha } = require('foo');
), error messages will contain both the original name and the alilas for preventing confusion:`Alpha` import should occur before import of `Alpha as Bravo`
named
or thenamed.enabled
option can be set totrue
. However,named.all
might be a more suitable option name (?)Related
import/named-order
: Enforce to sort specifiers of import, export, require #2553, the original proposalMerging this PR will fix #2553 and fix #1787