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

Suggestion: Refactor extract to type alias/typedef #23869

Closed
mhegazy opened this issue May 3, 2018 · 14 comments · Fixed by #30562
Closed

Suggestion: Refactor extract to type alias/typedef #23869

mhegazy opened this issue May 3, 2018 · 14 comments · Fixed by #30562
Labels
Committed The team has roadmapped this issue Domain: JSDoc Relates to JSDoc parsing and type generation Domain: Refactorings e.g. extract to constant or function, rename symbol Fixed A PR has been merged for this issue Suggestion An idea for TypeScript

Comments

@mhegazy
Copy link
Contributor

mhegazy commented May 3, 2018

TypeScript Version: 2.9.0-dev.201xxxxx

Search Terms: Refactor, extract type, typedef

In a TypeScript file Extract to type alias:

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

would generate:

type newType = { a: number, b: string }
var x: newType = { .. };

In a JavaScript file Extract to typedef:

/** @type {import("./c2").mytype} */
var x;

would generate:

/** @typedef {import("./c2").mytype} newType */

/**@type {myType} */
var x;

gif courtesy of @DanielRosenwasser

extracttypealias

@mhegazy mhegazy added Suggestion An idea for TypeScript Domain: Refactorings e.g. extract to constant or function, rename symbol Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Domain: JSDoc Relates to JSDoc parsing and type generation labels May 3, 2018
@Kingwl
Copy link
Contributor

Kingwl commented Jun 11, 2018

it's very helpful to me

@mhegazy
Copy link
Contributor Author

mhegazy commented Jun 11, 2018

@Kingwl is that something you would be interested to implement as well? we would accept a PR for this one.

@Kingwl
Copy link
Contributor

Kingwl commented Jun 11, 2018

sure,but it going to be late,my deadline is coming😂

@Kingwl
Copy link
Contributor

Kingwl commented Jun 19, 2018

something need to consider:

  1. option parameter?
  2. rest parameter?
  3. update all CallExpression ?

@mhegazy
Copy link
Contributor Author

mhegazy commented Jun 19, 2018

  • For optional parameters I would strip off the |undefined from the type e.g. function f(a? :number | undefined) => type newType = number ; function f(a?: newType)
  • initializers need to be maintained, e.g. function f(a : number | string = 0) should be type newType = number | string; function f(a: newType = 0)
  • rest parameters are fine, since you are extracting their type
  • do not think you need to update call expressions..

I think you are confusing this feature with extract to named arguments. e.g. function f(a: number, b:string) => function f({a, b}: {a: number, b:string}). this one is tracked by #23552.

@Kingwl
Copy link
Contributor

Kingwl commented Jun 19, 2018

Yes, I confused the two feature (:sad) , this one looks like this is a relatively simple operation

@Kingwl
Copy link
Contributor

Kingwl commented Jun 21, 2018

could you give some advice about the new name of the newType?

@mhegazy
Copy link
Contributor Author

mhegazy commented Jun 21, 2018

dose not matter what name you pick really. it has to be unique. The new name will be the rename location for the refactoring, and thus the user will get to update it immateriality after the refactor is applied.

@Kingwl
Copy link
Contributor

Kingwl commented Jun 21, 2018

should it trigger with signal primitive type and extract to a type alias?

@mhegazy
Copy link
Contributor Author

mhegazy commented Jun 21, 2018

I suppose so.. any type node really should be extractable..

@RyanCavanaugh RyanCavanaugh added Committed The team has roadmapped this issue and removed Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature labels Mar 22, 2019
@RyanCavanaugh
Copy link
Member

@Kingwl we were just wishing we had this 😉

@Kingwl
Copy link
Contributor

Kingwl commented Mar 23, 2019

I'll on it😂

@mattwelke
Copy link

mattwelke commented Mar 31, 2019

The 3.4 release notes mention this issue for providing feedback about this feature were they to expand it to create a type for the new parameter object.

I feel like a nice convention would to be to tap into the name of the function and its class if the function is a method to arrive at the following convention: FunctionNameOptions OR ClassNameMethodNameOptions.

Example:

Before refactor:

class Foo {
    bar(a: string, b: number) {
      console.log('bar');
    }
}

Refactored:

interface FooBarOptions {
    a: string;
    b: number;
}

class Foo {
    bar(o: FooBarOptions) {
        console.log('bar');
    }
}

Instead of Options, the suffix could also be things like Params, Args, etc. Whatever the community thinks makes the most sense.

The parameters interface would be placed in the same file because it would make sense for another module to import this module in order to reference its types and use its functions. The developer could move the interface into another file if they wanted to.

@DanielRosenwasser DanielRosenwasser added this to the TypeScript 3.5.0 milestone May 14, 2019
@DanielRosenwasser DanielRosenwasser added the Fixed A PR has been merged for this issue label May 14, 2019
@DanielRosenwasser
Copy link
Member

Thanks @Kingwl!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Committed The team has roadmapped this issue Domain: JSDoc Relates to JSDoc parsing and type generation Domain: Refactorings e.g. extract to constant or function, rename symbol Fixed A PR has been merged for this issue Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants