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

Extract into simple component refactoring #2496

Merged
merged 21 commits into from Jul 21, 2023

Conversation

zardoy
Copy link
Contributor

@zardoy zardoy commented Mar 8, 2023

ref #2495

I'm attaching gif so you can easier to understand what is happening there:

GIF

code

Please let me know wether you are interested in the idea itself (and probably having some help here ;)

Updated questions:

  • I didn't find a way to get needed virtualFile mapping for specific interpolation range (eg v-bind directive). When I figure it out I will remove templateFormatScript usage in favor of templateAst so there is a better detection wether identifier should be written to emits. Sorry, for me its really hard to understand what is happening in template's codegen 😆
    In other words I currently map over generatedRanges and filter only identifiers and while it works in most cases I think it can give unpredictable results.
  • It seems that edit.changes cant be used along with documentChanges, is this intentional? So how to update current file and create a new one with code action then? On the other hand I think we should ask a name for component before, so it means moving to command with arguments like edits and addImportPosition. Or we can ask for component name after its creation and do file rename then for now.
  • I guess this is fine if I use ChangesTracker from TS API? For context: it is not exposed with typings, but TS code actions using it. It is needed for better formatting with factory API and handling import fixes.

todo:

  • handle import adding for types, like TS infer from usages do
  • support options API

Also if you're happy with idea, settings should be added to allow configure creating new components (e.g. force composition or options API to use)

@zardoy zardoy mentioned this pull request Mar 19, 2023
@johnsoncodehk
Copy link
Member

johnsoncodehk commented Mar 20, 2023

Hi, can you update the questions if their status has changed? I will answer later.

@zardoy
Copy link
Contributor Author

zardoy commented Mar 20, 2023

Hi, can you update the questions if their status has changed? I will answer later.

Done.

It also would be nice to normalize generated \t, but I will do it like volarjs/volar.js#20 is done, once issue with creating files from code action is resolved

@johnsoncodehk
Copy link
Member

  1. It seems a same question to Add v-bind code action #2524 (comment)?
  2. We have no restriction to use edit.changes and documentChanges at the same time, probably a limitation of the IDE itself. I prefer to add a rename file command to the code action.
  3. Can you share some references about ChangesTracker? I don't find it in TypeScript source code.

The current code is a bit difficult to review. It would be great if it can be abstracted into multiple functions according to the intention. I will be happy to help realize it.

@hkochniss
Copy link

@zardoy
hope this doesn't go to "sleep". This is literally the one refactor that's missing from Volar (in comparison to let's say WebStorm)

@zardoy
Copy link
Contributor Author

zardoy commented Apr 21, 2023

@hkochniss For vscode specifically you can check Vue Extract Component

As you noticed I currently don't work on this, this pr requires some debugging & issues resolving. Probably going to spend some time later

@johnsoncodehk
Copy link
Member

johnsoncodehk commented Apr 21, 2023

I will look into this and #2524 in v1.5.x v1.7.x. (Not far, v1.5 is just the next pre-release version.)

@johnsoncodehk johnsoncodehk marked this pull request as ready for review July 21, 2023 15:53
@johnsoncodehk
Copy link
Member

I think the functionality is working fine now, let's merge it.

@johnsoncodehk johnsoncodehk linked an issue Jul 21, 2023 that may be closed by this pull request
@johnsoncodehk johnsoncodehk merged commit b7fd537 into vuejs:master Jul 21, 2023
3 checks passed
@johnsoncodehk
Copy link
Member

  • handle import adding for types, like TS infer from usages do

We can try setting addMissingImports command for codeAction.command to do it.

  • support options API

Let's ditch the options api for this functionality, the current workload is already a bit unmaintainable.

@johnsoncodehk
Copy link
Member

Options API is also supported by f012a57, I found that we already have the code to modify the components option.

@zardoy
Copy link
Contributor Author

zardoy commented Jul 27, 2023

Options API is also supported by f012a57, I found that we already have the code to modify the components option.

omg thank you so much, now this code action would save a lot of time in big legacy code bases that use options api!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Idea: Extract to new SFC component refactoring
3 participants