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.

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] extensions: allow enforcement decision overrides based on specifier #3105

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Xunnamius
Copy link
Contributor

@Xunnamius Xunnamius commented Nov 19, 2024

This PR implements in import/extensions: a new option enabling developers to override the decision to enforce or ignore extensions for imports with matching specifiers.

A demo package containing this feature is temporarily available for easy testing:

npm install --save-dev eslint-plugin-import@npm:@-xun/eslint-plugin-import-experimental

Prevent false negatives when checking the extensions of bespoke imports

I already use import/order with custom pathGroups. This allows me to control the grouping of certain paths, such as custom aliases from tsconfig.json's paths. This in turn ensures all imports are sorted properly, even when the project is using syntactically sound but otherwise bespoke custom aliases like my-project:api.js (in lieu of ../../../../src/api.js).

Unfortunately, if I mistype my-project:api (i.e. I forgot the extension), the import/extensions rule will still pass; the enforcement decision is "ignore" when it should have been "enforce". This is because, like import/order, the grouping strategy employed by this rule is quite coarse-grained, but unlike import/order, this rule is missing a pathGroups-like setting for finer-grained sorting. The goal of this PR is to fill that gap.

The documentation updates associated with the changes in this PR are part of a separate PR and can be previewed here (towards the bottom of the section). This PR also includes tests.

Copy link

codecov bot commented Nov 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.64%. Comparing base (1dc101b) to head (a0ab04d).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3105      +/-   ##
==========================================
+ Coverage   90.45%   94.64%   +4.19%     
==========================================
  Files          84       84              
  Lines        3634     3644      +10     
  Branches     1279     1284       +5     
==========================================
+ Hits         3287     3449     +162     
+ Misses        347      195     -152     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ljharb ljharb force-pushed the contrib-path-group-overrides branch from 1c127b7 to b067170 Compare December 11, 2024 19:37
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if two contradictory pathGroupOverrides options end up applying to the same file?

@ljharb ljharb marked this pull request as draft December 11, 2024 19:38
@ljharb ljharb changed the title feat(import/extensions): allow enforcement decision overrides based on specifier [New] extensions: allow enforcement decision overrides based on specifier Dec 11, 2024
@Xunnamius
Copy link
Contributor Author

What happens if two contradictory pathGroupOverrides options end up applying to the same file?

It mirrors the behavior of pathGroups from import/order: the first path group with a matching minimatch pattern wins.

@Xunnamius Xunnamius force-pushed the contrib-path-group-overrides branch from b067170 to 344df04 Compare December 23, 2024 02:20
@Xunnamius Xunnamius marked this pull request as ready for review December 23, 2024 03:09
@Xunnamius
Copy link
Contributor Author

@ljharb Did this PR need any further work?

@ljharb
Copy link
Member

ljharb commented Dec 23, 2024

@Xunnamius there's a few lines in the diff that aren't covered by tests.

@ljharb ljharb marked this pull request as draft December 23, 2024 04:42
@Xunnamius Xunnamius force-pushed the contrib-path-group-overrides branch from 344df04 to 7a69297 Compare December 23, 2024 20:53
@Xunnamius
Copy link
Contributor Author

Xunnamius commented Dec 24, 2024

@ljharb I'm not seeing any uncovered lines on codecov? Though I did see some failing eslint tests. They are now passing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants