Skip to content

Shrink span for convert-to-es6-module suggestion #23441

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
1 commit merged into from
Apr 16, 2018

Conversation

ghost
Copy link

@ghost ghost commented Apr 16, 2018

Fixes #23272

@ghost ghost requested a review from amcasey April 16, 2018 19:49
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.

Did it used to be large because it was convenient when it was a refactoring? Or was it just an oversight?

@@ -3,7 +3,7 @@
// @allowJs: true

// @Filename: /a.js
////[|exports.f = function() {}|];
////[|exports.f|] = function() {};
////exports.C = class {};
Copy link
Member

Choose a reason for hiding this comment

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

Do we only offer the fix on the first one?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, only at the single location that comes from commonJsModuleIndicator.

@ghost
Copy link
Author

ghost commented Apr 16, 2018

When it was a refactoring we used completely different code, since we started with a range and had to determine whether it was highlighting a commonjs-y thing. Now we are walking through the source file and adding suggestion diagnostics.
So to answer your question, there was only an error span when this was recently converted to a suggestion diagnostic instead of a refactoring.

@ghost ghost merged commit 40fd6ae into master Apr 16, 2018
@ghost ghost deleted the getErrorNodeFromCommonJsIndicator branch April 16, 2018 23:46
@microsoft microsoft locked and limited conversation to collaborators Jul 26, 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.

1 participant