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

support SFC script with setup #41

Conversation

ztiromoritz
Copy link

This should be backward compatible and support <script setup> Tags as well.
Not 100% sure about using both tags in one file (with and without setup) but I think this would be an edge case.

@simonhaenisch
Copy link
Owner

Thanks for the PR, I'll try to have a look at this soon, can't make any promises though.

@Shinigami92
Copy link

Could you also add a test for <script setup lang="ts">?

@Shayan-To
Copy link

I tested this PR, and it had a bug. I had a component imported in my script setup block, and had used it in my template. But it was removed when I ran prettier.
@ztiromoritz Do you have any ideas what the problem could be?

@Shinigami92
Copy link

@Shayan-To Can you provide your Vue file? Then we could add the content as a test case.

@ztiromoritz
Copy link
Author

ztiromoritz commented Jan 10, 2022

Could you also add a test for <script setup lang="ts">?

@Shinigami92 Hi, I added a simple testcase including ts

I tested this PR, and it had a bug. I had a component imported in my script setup block, and had used it in my template. But it was removed when I ran prettier. @ztiromoritz Do you have any ideas what the problem could be?

@Shayan-To Do you use an component import plugin in your build setup? We are using vite and "unplugin-vue-components". I think vue-cli setups often have similar mechanisms. (https://www.npmjs.com/package/vue-cli-plugin-import-components). With such a plugin the components you use in your template don't have to be imported in your script.

As I understand, this is a general limitation of 'prettier-plugin-organize-imports' because it orderes the imports of the code isolated in the specifict script tag. So there are no informations involved about the components you might use in your template. Therefore they will be removed as unused imports. So it only works together with an import component plugin.
@Shinigami92 Please correct me, if I'm wrong.

( see also #37 )

@Shayan-To
Copy link

I don't like to use vue-cli-plugin-import-components. It does "magic" and your code works, but the magic is really not needed when all you need is a single import and nothing more.

VSCode's organize imports command does the job correctly, and it seems to work for specific script tags (you have to place your cursor inside the script tag, and it only organizes the imports of that specific script tag). All the data is inside the vue file, the problem is that this PR does not take the usages in the template into account.

If I want to provide an example of an import that should not be removed:

<template>
  <MyComponent />
</template>
<script setup lang="ts">
  import MyComponent from "./MyComponent.vue";
</script>

@Shayan-To
Copy link

Shayan-To commented Jan 10, 2022

I ran this PR against my repo again, and I was reminded that there can be other things that are only used in the template, and will be removed when organizing imports using this PR.

Example:

<template>
  <some-input :validator="validateSomething" />
</template>
<script setup lang="ts">
  import { validateSomething } from "src/utils/validators";
</script>

@ztiromoritz
Copy link
Author

I think those are very valid concerns about how this library handles components and other imports used only in the template.
But I think that is not the point of this specific PR.

The use case you describe won't work with plain script tags either:
https://github.com/ztiromoritz/prettier-plugin-organize-imports/blob/component-imports/test.js#L138-L151
(This test case fails as well.)

@Shinigami92
Copy link

I would love to have this merged, just a progress, step by step

@simonhaenisch
Copy link
Owner

simonhaenisch commented Jun 17, 2022

Superseded by #56.

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.

None yet

4 participants