Skip to content

Feat: Extract function with jsx selection should be replaced with jsx sfc #25891

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

Open
cevek opened this issue Jul 24, 2018 · 6 comments
Open
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Domain: Refactorings e.g. extract to constant or function, rename symbol Suggestion An idea for TypeScript

Comments

@cevek
Copy link

cevek commented Jul 24, 2018

Suggestion

If we select jsx element and do extract refactor, editor should be refactor it in jsx maner

Examples

function Foo({ items }: { items: { id: number; name: string }[] }) {
    return <div>{items.map(item => <div>{item.id}: {item.name}</div>)}</div>;
}

After refactor should be

function Foo({ items }: { items: { id: number; name: string }[] }) {
    return <div>{items.map(item => <Bar id={item.id} name={item.name}/>)}</div>;
}

function Bar(item: { id: number; name: string; }) {
    return <div>{item.id}: {item.name}</div>;
}

Current behavior

ezgif-5-b8e179a53e

@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 labels Jul 24, 2018
@mhegazy
Copy link
Contributor

mhegazy commented Jul 24, 2018

I would say a new refactoring should be offered in this context to extract the JSX element to an SFC.

@cevek
Copy link
Author

cevek commented Jul 24, 2018

But source code of new refactoring will be almost the same as main extractFunction refactoring, isn't it? Or we can reuse existent code?

@mhegazy
Copy link
Contributor

mhegazy commented Jul 24, 2018

This has nothing to do with the source code. the same refactoring implementation can offer multiple refactorings to the user. for instance extractFunction offers one for every containing scope. it is still the same implementation, it just gives the user choices.

@mariusGundersen
Copy link

I think this would be more obvious and useful when selecting a chunk of jsx, for example:

function Component({title, subtitle, author, publishedDate, text}){
  return (
    <div>
      <h1>{title}</h1>
      <h2>{subtlitle}</h2>
      <div className="byline">
        <a href={author.url}>{author.name}</a> ({publishedDate})
      </div>
      <p>{text}</p>
    </div>
  )
}

When selecting and extracting the .byline div and extracting it, it should be replaced with the jsx call to the function: like this:

function Component({title, subtitle, author, publishedDate, text}){
  return (
    <div>
      <h1>{title}</h1>
      <h2>{subtlitle}</h2>
-      <div className="byline">
-        <a href={author.url}>{author.name}</a> ({publishedDate})
-      </div>
+      <Byline author={author} publishedDate={publishedDate} />
      <p>{text}</p>
    </div>
  )
}

For this to work correctly it's necessary to make the function accept only a single parameter, so that it can be sent in as the props. That is, the implementation would in the above scenario be

function Byline({author, publishedDate}){
  return (
    <div className="byline">
      <a href={author.url}>{author.name}</a> ({publishedDate})
    </div>
  )
}

@OliverJAsh
Copy link
Contributor

OliverJAsh commented May 12, 2021

Perhaps we could achieve this by adding a few smaller refactorings and then composing them together to create a larger refactoring.

  1. Extract to function in module scope
  2. Convert parameters to destructured object
  3. Convert call expression to JSX element

Here's a video where I demonstrate this using the example above: https://www.loom.com/share/4498fb6487bf42c39aa59a4a52dee5a1

The first 2 refactorings already exist, so we'd just need to add the third one ("Convert call expression to JSX element"). (I have written a version of this myself which I'm happy to share if anyone is interested.)

Once we have that, TypeScript could (somehow) package up these refactorings into a larger one and call it something like "Extract JSX as component".

At my workplace we prefer to write components using function expressions instead of function declarations. This has a few extra steps:

  1. Extract to function in module scope (as before)
  2. Convert parameters to destructured object (as before)
  3. Convert function declaration to arrow function
  4. Annotate as React.FC
  5. Convert call expression to JSX element (as before)

https://www.loom.com/share/01281c815f5b44d38b3774df37ce703a

@OliverJAsh
Copy link
Contributor

Note this is very similar to #15090.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Domain: Refactorings e.g. extract to constant or function, rename symbol Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants