microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101k stars 12.48k forks source link

Refactor: Generate narrower type based on usage. #47896

Open ericwooley opened 2 years ago

ericwooley commented 2 years ago

Suggestion

πŸ” Search Terms

  1. refactor
  2. type
  3. narrow
  4. generate
  5. interface
  6. decouple

βœ… Viability Checklist

My suggestion meets these guidelines:

⭐ Suggestion

A new refactor feature which creates a minimal interface based on usage for an argument.

πŸ“ƒ Motivating Example

Imagine we have created an interface, User.

interface User {
    id: string,
    created_at: Date,
    updated_at: Date
    name: string
    email: string
}

When we started with our project, we created some super fancy rendering table which shows a list of users by name, and when they were last_updated, and when they were created.

const renderTable = (data: User[])=> {
  return (
    <table>
      <thead> <th key={i.id}><td>name</td><td>created at</td><td>updated at</td></th></thead>
      <tbody>
        {data.map(i => <tr key={i.id}><td>{i.name}</td><td>{i.created_at}</td><td>{i.updated_at}</td></tr>)}
      </tbody>
    </table>
  )
} 

Then we decide to add books to our app.


interface Book {
   id: string
   name: string
   created_at: Date
   updated_at: Date
   last_checkout: {date: Date, user: User}
}

We want the same functionality for rendering books as we have for users. Luckily, renderTable is pretty re-usable. So we just add our interface to types for data:

const renderTable = (data: (User|Book)[])=> {
// ...

Then, we add Libraries, and repeat

const renderTable = (data: (User|Book|Library)[])=> {
// ...

We could keep adding to this list, but in order to avoid infinite joins, we might want to refactor this, so that the data type only specifies what it actually needs. So we create a generic interface, maybe called TableRenderable, which has all the properties we need to make the data renderable.

interface TableRenderable {
  id: string
  name: string
  created_at: Date
  updated_at: Date
}
const renderTable = (data: TableRenderable[]) => {

Nice, now we have a minimal interface that this table needs to be able to render the data.

That was easy enough, but imagine if table weren't so simple, maybe it called half a dozen other functions, which all use the User interface, and had a few more than 3 properties.

In order to refactor, you would need to first go into each function, and create the minimal interface for each function, then on the renderTable function, create the minimal interface TableRenderable. This still isn't that bad, but it is a bit time consuming.

Typescript can help by implementing a refactor feature Generate Minimal Interface. Typescript already has all the information needed in the function to know which properties are being used by this function, and what types the are. We just need to combine all that information into a new interface. Tedious for human beings, simple for a type system.

Now with the new feature, you can start with each function renderTable calls, replace the User type with a generated minimal interface. Then finally, in the renderTable function, create the TableRenderable interface.

Instead of 10+ minutes in many of my real use cases, decoupling functions from the User interface could take a minute or 2.

πŸ’» Use Cases

What do you want to use this for?

Decoupling interfaces from functions much more quickly.

What shortcomings exist with current approaches?

Decoupling in this manner is manual, as far as I am aware, which is ok, but it's something that a computer could do just as easily.

RyanCavanaugh commented 2 years ago

Just to make sure I understood this properly, the idea is that the user would select User | Book | Library and be offered the option to generate the interface, and that interface would contain every property which exist in all of the constituents of the union?

ericwooley commented 2 years ago

Just to make sure I understood this properly, the idea is that the user would select User | Book | Library and be offered the option to generate the interface, and that interface would contain every property which exist in all of the constituents of the union?

Correct, except I'm not exactly sure if we're on the same page about this part:

every property which exist in all of the constituents of the union?

It wouldn't be every property in the constituents of the union, just the properties which are used by this method/function

RyanCavanaugh commented 2 years ago

Hmm, interesting. What you're describing isn't very far off from the existing "Infer parameter types from usage" refactor that appears when a parameter doesn't have a type annotation.

ericwooley commented 2 years ago

Hmm, interesting. What you're describing isn't very far off from the existing "Infer parameter types from usage" refactor that appears when a parameter doesn't have a type annotation.

Hmm, i haven't seen that? Under what circumstances does that appear. I do not see it when running refactor on an argument with implicit any. I just see these options in vs code with typescript 4.5.5

image

fatcerberus commented 2 years ago

@ericwooley image

Result is:

function fooey(foo: { x: number; y: string; })
{
    const x: number = foo.x;
    const y: string = foo.y;
}
ericwooley commented 2 years ago

@fatcerberus

Oh nice, thanks! I never noticed that, I guess I just always write my types out before I start writing the body 🀷

I tried playing with that in the TS playground to see if it would be sufficient help in the User example, and the best I could get it to infer was any[]. Which isn't surprising, but I wanted to be sure I wasn't missing things

Playground image

ericwooley commented 2 years ago

@RyanCavanaugh Is there anything I need to add to get past the "Awaiting More Feedback" label?

RyanCavanaugh commented 2 years ago

@ericwooley no, AMF is sort of the default state for suggestions that don't need design work but don't currently meet the bar for adding to the codebase