Skip to content

In import fixes, use a ".js" extension if other imports do #20624

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

Merged
2 commits merged into from
Jan 3, 2018

Conversation

ghost
Copy link

@ghost ghost commented Dec 11, 2017

Fixes #19019

@ghost ghost force-pushed the importNameCodeFix_jsExtension branch from 15f851e to fffc5f9 Compare December 11, 2017 18:28
@ghost ghost requested a review from amcasey December 18, 2017 19:09
@ghost ghost force-pushed the importNameCodeFix_jsExtension branch from fffc5f9 to 7588c15 Compare January 2, 2018 20:05
Copy link
Member

@amcasey amcasey left a comment

Choose a reason for hiding this comment

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

Seems reasonable. Our expectation is that users will manually correct the first generated input in a file and that things will work correctly thereafter?

return addJsExtension
? noExtension + ".js"
: getEmitModuleResolutionKind(options) === ModuleResolutionKind.NodeJs
? removeSuffix(noExtension, "/index")
Copy link
Member

Choose a reason for hiding this comment

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

I don't know the JS/TS convention, but I'm using to seeing these lines indented.

////export function b() {}

// @Filename: /c.ts
////import { a } from "./a.js";
Copy link
Member

Choose a reason for hiding this comment

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

For documentation purposes, it might be nice to add a test where some imports have extensions and others do not. (It looks like absolute paths might also be treated specially.)

@ghost
Copy link
Author

ghost commented Jan 3, 2018

Ideally we would have an option for this (#20619) but yeah, for now we will rely on the user manually correcting the first import.

@ghost ghost merged commit 74b8160 into master Jan 3, 2018
@ghost ghost deleted the importNameCodeFix_jsExtension branch January 3, 2018 16:20
@microsoft microsoft locked and limited conversation to collaborators Jul 3, 2018
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants