Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat(extensions): enhance import extension enforcement with autofix support#3177

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
stephenjason89 wants to merge2 commits intoimport-js:main
base:main
Choose a base branch
Loading
fromstephenjason89:feat/extensions-autofix

Conversation

stephenjason89
Copy link

@stephenjason89stephenjason89 commentedApr 8, 2025
edited
Loading

Summary

This PR enhances the import extensions rule by introducing auto-fix support. With this change, ESLint can now automatically:

  • Append a missing file extension when one is required.
  • Remove an extension when its presence is forbidden and the module is resolvable without it.

Changes

  • New Configuration Option:
    Added a newfix option (defaulting tofalse) in the rule's configuration. When enabled, the rule will attempt to fix detected issues automatically.

  • Meta Update:
    Marked the rule as fixable by addingfixable: 'code' in the meta information.

  • Auto-fix Implementation:
    Extended the reporting logic incheckFileExtension with fixer functions:

    • For missing file extensions, if auto-fix is enabled, the fixer appends the detected extension.
    • For extraneous file extensions, the fixer removes the extension from the import statement.
  • Code Adjustments:
    Updated thebuildProperties function to handle the newfix property from the user configuration.

Motivation

This update improves developer experience by allowing ESLint to automatically correct common mistakes regarding file extensions in import paths, reducing manual intervention during code review or development.

Backward Compatibility

All existing functionality remains unchanged when thefix option is disabled. Users can enable auto-fixing by settingfix: true in their ESLint configuration for this rule.

jimmywarting and stephenjason89 reacted with thumbs up emojilnhrdt, ezfe, and stephenjason89 reacted with hooray emoji
Copy link
Member

@ljharbljharb left a comment

Choose a reason for hiding this comment

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

Can you confirm you did not use an LLM to generate any of this code? LLM-generated code has unclear provenance and can't be safely licensed to an open source project.

@stephenjason89
Copy link
Author

Can you confirm you did not use an LLM to generate any of this code? LLM-generated code has unclear provenance and can't be safely licensed to an open source project.

@ljharb I've reverted the parts where it was LLM generated. Didn't know what to put in type, category, and description for meta. Also, my PR description is LLM generated, i have reviewed it though, I hope that's alright.

The rest is my work.

Thanks for taking the time to review this PR! I'm a bit stuck on the remaining failing tests and not sure how to proceed. Would you be open to taking ownership of this PR to get it across the finish line?

Again, thank you!

ljharb reacted with thumbs up emoji

@ljharb
Copy link
Member

Thanks for confirming.

The test failures are because your changes have broken the rule's behavior. One thing to look at is that untilresolve supportsexports, we can only safely autofix relative/local files.

I can't take ownership of this as-is, unfortunately.

stephenjason89 reacted with thumbs up emoji

@stephenjason89stephenjason89force-pushed thefeat/extensions-autofix branch 3 times, most recently from6f11cda toe5443f8CompareApril 8, 2025 20:16
@stephenjason89
Copy link
Author

stephenjason89 commentedApr 8, 2025
edited
Loading

@ljharb Thanks for the reply. I have further reduced the failing tests to 12 now. I also need your feedback on reimplementinggetModifier since it falls back toprops.defaultConfig which isnever.

Problem is, if we add a rule like

       "import/extensions": ["error", {                "pattern": {                    "js": "always",                },            }],

All the other extensions not defined in pattern falls back tonever which is not ideal. Ifts ,for example, is not defined then i would expect it to behave without this rule applied. I should explicitly opt in to this rule by definingalways ornever.

Please help on the remaining failing tests as well. I'm a bit stuck on the remaining tests.

Thank you

ljharb reacted with eyes emoji

@codecovCodecov
Copy link

codecovbot commentedApr 9, 2025
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.29%. Comparing base(da5f6ec) to head(b8e7370).

Additional details and impacted files
@@            Coverage Diff             @@##             main    #3177      +/-   ##==========================================- Coverage   95.52%   95.29%   -0.24%==========================================  Files          83       83                Lines        3690     3695       +5       Branches     1333     1337       +4     ==========================================- Hits         3525     3521       -4- Misses        165      174       +9

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

🚀 New features to boost your workflow:
  • ❄️Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@stephenjason89stephenjason89force-pushed thefeat/extensions-autofix branch 3 times, most recently from49381fd toe6458bfCompareApril 9, 2025 17:58
@stephenjason89
Copy link
Author

@ljharb, I’ve reworked my pull request to include afix prop for backward compatibility.
The first implementtion where autofixing was the default behavior was triggering several test failures—I’ve resolved nearly all of them, with nine remaining. But attempting to resolve them all bloated the PR.

This new version should be much more targeted and should be easier to review.

ljharb reacted with eyes emoji

@stephenjason89
Copy link
Author

@ljharb I've rebased and added tests to increase coverage. Please re-review and merge if it looks good.

Thanks!

solcik reacted with thumbs up emoji

@stephenjason89
Copy link
Author

@ljharb may I gently follow up on this PR, would love to hear you feedback.

Thanks

solcik, lucgagan, ljharb, and davidenke reacted with eyes emoji

@lucgagan

This comment was marked as spam.

@ljharb

This comment was marked as off-topic.

fix(fixer) {
return fixer.replaceText(
source,
JSON.stringify(`${importPathWithQueryString}.${extension}`),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seehttps://github.com/un-ts/eslint-plugin-import-x/pull/329/files#r2094349587

JSON.stringify is not correct for single quoted imports.

https://github.com/un-ts/eslint-plugin-import-x/pull/327/files#r2094298890

importPathWithQueryString could containsquery andhash, and could also ends with.,.. or/.

return fixer.replaceText(
source,
JSON.stringify(
importPath.slice(0, -(extension.length + 1)),
Copy link
Collaborator

Choose a reason for hiding this comment

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

@ljharbljharb marked this pull request as draftJune 20, 2025 15:47
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@JounQinJounQinJounQin requested changes

@ljharbljharbAwaiting requested review from ljharb

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@stephenjason89@ljharb@lucgagan@JounQin

[8]ページ先頭

©2009-2025 Movatter.jp