Skip to content

Cases for refactor discoverability #37895

Closed

Description

Refactoring cases for which we can improve discoverability

This is the list of our refactors TSServer offers and the proposed changes to make them more discoverable. See #34930 for the broader discussion.

Here I have listed the current behavior as well as proposed changes to make the refactors more discoverable. Some of these ideas may offer refactors too frequently/infrequently, so more input is desired. The explicit refactoring request cases mentioned depend on #35096.

This issue does not list any code fixes.

addOrRemoveBracesToArrowFunction

Case - remove braces:

const a = (n: number) => {
    return n;
}

Current:

  • Refactor offered for cursor or selection start in range (n: number). selection end can be anywhere else.

Proposed:

  • Explicit request offers refactor for cursor or selection anywhere in function including body.
  • Implict refactor seems OK as is.

convertExport

Case - Convert default export to named export:

export default function f();

Case - Convert named export to default:

export function f();

Current:

  • Refactor offered only when entire range is selected.

Proposed:

  • Explicit request offers refactor for cursor or selection anywhere in range.
  • Implicit refactor triggers for cursor or selection anywhere in ranges f() or default.
  • Note this refactor only applies when the only statement in the function is a return statement, so we aren't in danger of offering up this refactor too much. We may even be fine offering it as often as in the explicit case.

convertImport

Case - Convert namespace import to named imports:

import * as a from "./a";

Case - Convert named import to namespace import:

import { f } from "./a";

Current:

  • Refactor offered only when entire range is selected.

Proposed:

  • Explicit request offers refactor for cursor or selection start anywhere in range. Implicit refactor triggers anywhere in * as a or { f }.

extractSymbol

Case - extract const

function foo(a: any) {
    return a + 20 * 10;
}

Current:

  • No refactor offered for cursor anywhere within statement.
  • Refactor offered for selection if selection start and end are within the same Literal. For example, selecting 2 offers to extract 20. This is not the case for other valid extraction candidates such as Identifiers or functions.
  • No refactor offered for selections if selection start and end are in the LHS and RHS of the same binary expression, but do not span it. For example, if 20 + 1 is selected, no refactor is offered.
  • See Improving refactoring discoverability and UX #34930.

Proposed:

  • Treat selections as though they span the nodes in which the ends terminate.
  • Treat an explicit refactor request for a cursor as though the largest node starting at that location is selected.

Case - extract method

function foo(){
    function bar(){
        return 0;
    }
}

Current

  • Refactor offered only if entire inner function is selected.

Proposed:

  • Implicitly offer extraction for functions for a cursor in the function name or function keyword (or both).
  • Offer refactor for selections if the start posisiton is anywhere in function bar.

extractType

Case - extract to type alias

var y: string = '';

Current:

  • Must have entire type string selected.

Proposed:

  • Implicitly treat partial selection of string as the selecting whole type.
  • Explicit refactor requests should offer the refactor if the cursor is in string

Case - extract to interface

var x: { a?: number, b?: string } = { };

Current:

  • Must have entire type literal { a?: number, b?: string } selected.

Proposed:

  • Offer refactor if cursor is at either brace location.

generateGetAccessorAndSetAccessor

Case - Generate 'get' and 'set' accessors

class A {
    public convertMe: string;
}

Current:

  • Refactor offered when selection start or end is in range convertMe.

Proposed:

  • Offer refactor on explicit request for cursor or selection start anywhere in the property declaration.

Refactors that are probably okay

Included for completeness. These cases are probably fine as is.

convertParamsToDestructuredObject

function f(a: number, b: number) {
    return a + b;
}

Current:

  • Refactor offered for cursor or selection start in range function f(a: number, b: number. Selection end can be anywhere else.

convertStringOrTemplateLiteral

const a = "as above, ";
const b = a + "so below";

Current:

  • Refactor offered for cursor or selection start in range a + "so below. Selection end can be anywhere else.

moveToNewFile

console.log("goodbye");

Current:

  • Must have entire statement selected.
  • Since so many things can be moved to a new file this refactor is already very common and we probably don't want to offer it up too much.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

Labels

CommittedThe team has roadmapped this issueDomain: Refactoringse.g. extract to constant or function, rename symbolDomain: TSServerIssues related to the TSServerRescheduledThis issue was previously scheduled to an earlier milestoneSuggestionAn idea for TypeScript

Type

No type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions