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

Add proper file extensions when importing a typescript file from a typescript file #20

Merged

Conversation

rosskevin
Copy link

@rosskevin rosskevin commented Jun 1, 2022

Problem

Given this configuration:

{
    "n/file-extension-in-import": ["error", "always"]
}

and file1.ts

import something from "./file2"

eslint will fix this with a .ts added to the import statement, which is invalid and breaks the compilation of typescript.

Solution

When a typescript file (.ts / .cts / .mts files, .cts and .mts have been supported since TypeScript 4.5) imports another typescript file, eslint will now use the proper matching file extension instead of the original referenced file extension.

This means that, for example, when a .ts file imports another .ts file, eslint will now suggest using a .js extension in the import statement.

Release

As far as I can tell, this should be good to release as a patch.

Credit

Mostly cherry-picked from https://github.com/giladgd/eslint-plugin-node/tree/dev/giladgd/fixImportExtentionFixingInTypeScript and lingering PR mysticatea#303

Thanks @giladgd

@rosskevin
Copy link
Author

@aladdin-add I don't want to be pushy, but any chance of a quick turnaround release? I'm migrating a huge codebase and struggling without this (and my private package attempts are currently failing on my private namespace).

{
filename: fixture("test.ts"),
code: "import './d.js'",
},

Choose a reason for hiding this comment

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

can you add a few invalid tests to verify the fixer(output)?

Copy link

@aladdin-add aladdin-add left a comment

Choose a reason for hiding this comment

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

LGTM, I'll just cut a release - the new added tests can be landed in another PR. :)

@aladdin-add aladdin-add merged commit c8d0484 into eslint-community:master Jun 2, 2022
@aladdin-add
Copy link

released in https://github.com/weiran-zsd/eslint-plugin-node/releases/tag/15.2.1 🎉

@calebeby
Copy link

@aladdin-add This behavior works in 15.2.1 but does not work in 15.2.2. In 15.2.2, the .ts extension is inserted in a TS file importing another TS file instead of inserting a .js extension. I think #24 broke this behavior.

cc @rosskevin

@rosskevin
Copy link
Author

I'm looking at it now. I thought I added the proper valid tests but it doesn't look like it (or I just got mixed up in the process).

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

Successfully merging this pull request may close these issues.

None yet

3 participants