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

Extension not working for .vue files #133

Closed
albertoxamin opened this issue Jul 14, 2020 · 9 comments · Fixed by #157
Closed

Extension not working for .vue files #133

albertoxamin opened this issue Jul 14, 2020 · 9 comments · Fixed by #157
Assignees
Labels
✨ Feature New refactoring or feature

Comments

@albertoxamin
Copy link

Is this request related to a problem? Please describe.

The extension does not work for vue files that contain both html/pug and js

Describe the solution you'd like

I would like the extension to recognize .vue files as js files

@albertoxamin albertoxamin added the ✨ Feature New refactoring or feature label Jul 14, 2020
@nicoespeon
Copy link
Owner

nicoespeon commented Jul 19, 2020

Thanks for pointing this out 👍

It would be cool indeed to handle .vue files like this:

<template>
  <div id="app">
    <h1>My Todo App!</h1>
    <TodoList/>
  </div>
</template>

<script>
import TodoList from './components/TodoList.vue'

export default {
  components: {
    TodoList
  }
}
</script>

<style lang="scss">
@import './variables.scss';

*, *::before, *::after {
  box-sizing: border-box;
}

#app {
  max-width: 400px;
  margin: 0 auto;
  line-height: 1.4;
  font-family: 'Avenir', Helvetica, Arial, sans-serif;
  -webkit-font-smoothing: antialiased;
  -moz-osx-font-smoothing: grayscale;
  color: $vue-blue;
}

h1 {
  text-align: center;
}
</style>

We currently parse the source code with babel here: 

babelParse(source, {
sourceType: "module",
allowImportExportEverywhere: true,
allowReturnOutsideFunction: true,
startLine: 1,
// Tokens are necessary for Recast to do its magic ✨
tokens: true,
plugins: [
"asyncGenerators",
"bigInt",
"classPrivateMethods",
"classPrivateProperties",
"classProperties",
// Not compatible with "decorators-legacy"
// "decorators",
"decorators-legacy",
"doExpressions",
"dynamicImport",
// Make tests fail, not sure why
// "estree",
"exportDefaultFrom",
"exportNamespaceFrom",
// Not compatible with "typescript"
// "flow",
// "flowComments",
"functionBind",
"functionSent",
"importMeta",
"jsx",
"logicalAssignment",
"nullishCoalescingOperator",
"numericSeparator",
"objectRestSpread",
"optionalCatchBinding",
"optionalChaining",
"partialApplication",
["pipelineOperator", { proposal: "minimal" }],
"placeholders",
"throwExpressions",
"topLevelAwait",
"typescript"
// Not compatible with "placeholders"
// "v8intrinsic"
]
})

This creates an AST we traverse and modify to refactor the code.

Unfortunately, it doesn't work with .vue files which can't be 100% turned into a JS AST. Thus, I don't have a great idea to make this work easily as of today. I can't find a babel parser plugin that would handle .vue extensions like we do for .jsx / .tsx.

@nicoespeon nicoespeon changed the title Extension not working for vue.js Extension not working for .vue files Jul 19, 2020
@nicoespeon nicoespeon added the 🙏 Help wanted Extra attention is needed label Jul 19, 2020
@nicoespeon
Copy link
Owner

nicoespeon commented Sep 3, 2020

Had a call with @OliverJAsh today and we discussed a way to solve that elegantly:

  1. Create a new Editor implementation (VueVSCodeEditor)
  2. Make it adapt to this situation so refactorings don't have to be touched
    • Get code from the <script> tags
    • Take the offset in consideration for Selection
    • Adapt refactored code to write back in the editor
  3. Create a getVSCodeEditorFromLanguageId(language_id: string): Editor to instantiate the correct editor (check where VSCodeEditor is instantiated)
    • Language ID can be retrieved from activeTextEditor.document
  4. Enable refactorings for Vue language ID (not sure what it is yet, but it's easy to find out)

image

@nicoespeon nicoespeon self-assigned this Sep 3, 2020
@nicoespeon nicoespeon removed the 🙏 Help wanted Extra attention is needed label Sep 4, 2020
@nicoespeon
Copy link
Owner

@all-contributors please add albertoxamin for ideas

@allcontributors
Copy link
Contributor

@nicoespeon

I've put up a pull request to add @albertoxamin! 🎉

@nicoespeon
Copy link
Owner

@albertoxamin this is now implemented 🎉

It will be part of the next release, which I planned to do next week. I'll let you know when it's out.

@nicoespeon
Copy link
Owner

Heads up @albertoxamin! This has been released with v4.8.0 🎉

Let me know if you run into troubles!

@leosdad
Copy link

leosdad commented Nov 6, 2020

Rename Symbol works fine with .js files but not with any .vue files. Other refactorings do work, but not Rename. Are there any settings I must activate for it to work?

@nicoespeon
Copy link
Owner

@leosdad yep, I realized that too. The reason is that Abracadabra's rename just fallbacks on VS Code's rename 😅

async function renameSymbol(editor: Editor) {
// Editor built-in rename works fine => ok to delegate the work for now.
await editor.delegate(Command.RenameSymbol);
}

It usually works very well. But turns out it doesn't work on Vue files.

I don't know how difficult it would be to implement. I think it wouldn't be trivial, but that could be an interesting one.

I can see that the issue has been closed in VS Code (microsoft/vscode#108854) and is still open in Vetur (vuejs/vetur#610)

We have other issues planned, but if that's important for you feel free to open an issue for this @leosdad. I think we could delegate to the editor and fallback on a custom implementation if it's not supported. Main logic would be to capture user input, find all other identical identifiers in scope and overwrite them with the new name.

@leosdad
Copy link

leosdad commented Nov 11, 2020

Too bad, I've installed Abracadabra in the hope it would solve this very limitation. Vue is still way behind other frameworks in terms of tooling in VS Code -- Vetur has several limitations and it's not evolving very much AFAIK. I'll start using your extension anyway, seems to be better than other ones I've tested and it's being actively developed.

I'll sure open an issue for this now. Many thanks for your detailed answer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ Feature New refactoring or feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants