Skip to content

add merge import declaration refactor#26575

Closed
Kingwl wants to merge 4 commits intomicrosoft:masterfrom
Kingwl:mergeDuplicateImport
Closed

add merge import declaration refactor#26575
Kingwl wants to merge 4 commits intomicrosoft:masterfrom
Kingwl:mergeDuplicateImport

Conversation

@Kingwl
Copy link
Copy Markdown
Contributor

@Kingwl Kingwl commented Aug 21, 2018

Fixes #26487

ajafff
ajafff reviewed Aug 21, 2018
View reviewed changes
Comment thread src/services/refactors/mergeImportDeclaration.ts Outdated
@@ -0,0 +1,94 @@
/* @internal */
namespace ts.refactor.addOrRemoveBracesToArrowFunction {
Copy link
Copy Markdown
Contributor

@ajafff ajafff Aug 21, 2018

Choose a reason for hiding this comment

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

That name should probably be amended

@RyanCavanaugh
Copy link
Copy Markdown
Member

RyanCavanaugh commented Sep 5, 2018

@uniqueiniquity @amcasey can you review the refactoring code please?

We should also figure out better phrasing; I don't like "duplicate" here.

@Kingwl
Copy link
Copy Markdown
Contributor Author

Kingwl commented Sep 7, 2018

I don't like "duplicate" here.

conflict?🙋🏻‍♂️

uniqueiniquity
uniqueiniquity approved these changes Sep 12, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@uniqueiniquity uniqueiniquity left a comment

Choose a reason for hiding this comment

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

How about "redundant" instead of "duplicate"?

@amcasey
Copy link
Copy Markdown
Member

amcasey commented Sep 17, 2018

Would this be more useful as a code fix? It doesn't seem like it would be excessively noisy and the transformation is one-way so the suggestion wouldn't immediately be replaced by its opposite.

@amcasey
Copy link
Copy Markdown
Member

amcasey commented Sep 17, 2018

Does this do something that Organize Imports doesn't do? If so, should Organize Imports be enhanced?

@RyanCavanaugh RyanCavanaugh added this to the TypeScript 3.2 milestone Sep 17, 2018
@Kingwl
Copy link
Copy Markdown
Contributor Author

Kingwl commented Sep 17, 2018

Would this be more useful as a code fix? It doesn't seem like it would be excessively noisy and the transformation is one-way so the suggestion wouldn't immediately be replaced by its opposite

Yes, but How could we provide the fix all action in this case?, maybe there are

Does this do something that Organize Imports doesn't do? If so, should Organize Imports be enhanced?

IMO: the error should be fix by a explicit behavior

@Kingwl
Copy link
Copy Markdown
Contributor Author

Kingwl commented Oct 16, 2018

⬆️

@Kingwl
Copy link
Copy Markdown
Contributor Author

Kingwl commented Jan 19, 2019

⬆️ again,
And i remember that why i donot use the quickfix:
import from same module is not an error

@Kingwl Kingwl closed this Mar 25, 2019
@Kingwl
Copy link
Copy Markdown
Contributor Author

Kingwl commented Mar 25, 2019

seems already existed

@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@ajafff ajafff ajafff left review comments

@uniqueiniquity uniqueiniquity uniqueiniquity approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@uniqueiniquity uniqueiniquity

@amcasey amcasey

Labels

None yet

Projects

None yet

Milestone

TypeScript 3.4.0

Development

Successfully merging this pull request may close these issues.

6 participants

@Kingwl @RyanCavanaugh @amcasey @uniqueiniquity @ajafff @DanielRosenwasser