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.

Sign up for GitHub

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

Jump to bottom

feat: support vue sfc #174

Merged
merged 2 commits into from Oct 20, 2022
Merged

feat: support vue sfc #174

merged 2 commits into from Oct 20, 2022

Conversation

blake-newman
Copy link
Contributor

resolves #49

@byara
Copy link
Collaborator

byara commented Oct 11, 2022

Hey @blake-newman thank you for your contribution. We will take a look 👍

@byara byara requested review from ayusharma and byara October 11, 2022 07:46
@blake-newman
Copy link
Contributor Author

I'm thinking may be best to abstract this in a better way to avoid conditions. Can have a separate processor. Going to push changes up shortly

@blake-newman blake-newman force-pushed the blake.newman/vue-support branch 2 times, most recently from c249762 to 82b2aa9 Compare October 11, 2022 09:48
byara
byara reviewed Oct 14, 2022
View reviewed changes
Copy link
Collaborator

@byara byara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution 🚀
I left some comments. It would be great if we could address them.

package.json Outdated
"jest": "26.6.3",
"prettier": "2.3.1",
"ts-jest": "26.5.3",
"typescript": "4.2.3"
"typescript": "4.2.3",
"vue": "^3.2.40"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is vue really needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No; i've removed 👍

src/preprocessor.ts Outdated
Comment on lines 10 to 26
export function preprocessor(code: string, options: PrettierOptions) {
if (options.filepath?.endsWith('.vue')) return code
return process(code, options)
}

export function vuePreprocessor(code: string, options: PrettierOptions) {
const { descriptor } = parse(code);
const content = (descriptor.script ?? descriptor.scriptSetup)?.content ?? code

return code.replace(
content,
`\n${process(content, options)}\n`,
);

}

function process(code: string, options: PrettierOptions) { {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest to make a preprocessor folder and separate them in different files. I would say process also can go in a separate file. Maybe preprocessor can also be renamed to something like defaultPreprocessor? What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah i've done the changes as requested. I'm happy with whatever you feel is best. Your repository after all :)

@ayusharma
Copy link
Collaborator

FYI: I will open and re-opening the branch to fix the Github CI issue.

@ayusharma ayusharma closed this Oct 20, 2022
@ayusharma ayusharma reopened this Oct 20, 2022
@blake-newman
Copy link
Contributor Author

@ayusharma i fixed the issue with the snapshot and also updated the README to reflect support for vue.

@ayusharma
Copy link
Collaborator

@ayusharma i fixed the issue with the snapshot and also updated the README to reflect support for vue.

Thank you so much, I will check it today.

ayusharma
ayusharma approved these changes Oct 20, 2022
View reviewed changes
Copy link
Collaborator

@ayusharma ayusharma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR looks good to me. 🎉 Very nice work and thank you so much for the contribution @blake-newman ❤️

I would suggest releasing this change with a new major version v4.0.0. What do you think @blake-newman and @byara ?

@ayusharma ayusharma requested a review from byara October 20, 2022 10:32
@blake-newman
Copy link
Contributor Author

Personally; i'd suggest even a patch/minor is fine as non-breaking and no changes in the configuration are needed from a user's perspective.

@ayusharma
Copy link
Collaborator

Personally; i'd suggest even a patch/minor is fine as non-breaking and no changes in the configuration are needed from a user's perspective.

Okay. I will release it with v3.4.0.

@byara byara merged commit cdf4565 into trivago:master Oct 20, 2022
@ayusharma
Copy link
Collaborator

Released in v3.4.0 🎉 Thank you again @blake-newman 😃

@tusharf5 tusharf5 mentioned this pull request Oct 23, 2022
Merged
@IanVS IanVS mentioned this pull request Oct 25, 2022
Merged
@ricardocosta
Copy link

@blake-newman @ayusharma Hello! First of all, thanks for adding the support to Vue files. I wanted to run something by you regarding this PR:

With Vue support, a dependency on @vue/compiler-sfc was added, which brings along quite a few other dependencies:

"@vue/compiler-sfc@npm:^3.2.40":
  version: 3.2.41
  resolution: "@vue/compiler-sfc@npm:3.2.41"
  dependencies:
    "@babel/parser": ^7.16.4
    "@vue/compiler-core": 3.2.41
    "@vue/compiler-dom": 3.2.41
    "@vue/compiler-ssr": 3.2.41
    "@vue/reactivity-transform": 3.2.41
    "@vue/shared": 3.2.41
    estree-walker: ^2.0.2
    magic-string: ^0.25.7
    postcss: ^8.1.10
    source-map: ^0.6.1

Do you see a way of supporting Vue, but keeping the dependency in a non-required way (e.g. peerDependency)? Non-Vue projects that use this plugin will now have that dependency installed for no reason.

@blake-newman
Copy link
Contributor Author

It may be possible to make it a peer dependency, may need to do inline require in preproccesor layer so lazy load the Vue compiler.

I'll take a look soon 👍

IanVS added a commit to IanVS/prettier-plugin-sort-imports that referenced this pull request Oct 26, 2022
@IanVS @blake-newman
Support Vue (#40)
bd83499 
Ported from trivago/prettier-plugin-sort-imports#174

Co-authored-by: Blake Newman <code@blakenewman.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ayusharma ayusharma ayusharma approved these changes

@byara byara byara approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Not sorting imports in .vue files
4 participants
@blake-newman @byara @ayusharma @ricardocosta