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

fix: prettier not working for svelte files in create-svelte projects #6866

Merged
merged 3 commits into from
Sep 19, 2022

Conversation

dominikg
Copy link
Member

fixes #6804

i'm not sure why this is needed again, it was previously removed in favor of the pluginSearchDirs setting in prettierrc.
Now we use both.

Note: the tests for create-svelte received 2 updates

  1. pin them to the same vite version kit uses to avoid type incompatibility errors with pnpm check during tests
  2. updated list of tested commands to include format, remove prepare and sort them differently

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

@changeset-bot
Copy link

changeset-bot bot commented Sep 17, 2022

🦋 Changeset detected

Latest commit: d55a8bf

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
create-svelte Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Comment on lines +6 to 7
"plugins": ["prettier-plugin-svelte"],
"pluginSearchDirs": ["."],
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should do

Suggested change
"plugins": ["prettier-plugin-svelte"],
"pluginSearchDirs": ["."],
"plugins": ["./node_modules/prettier-plugin-svelte"],

instead? I'm using this hack for all my projects which works well when running prettier through the CLI or VSCode extension. The --plugin-search-dir . can also be removed too.

I made a comment at sveltejs/prettier-plugin-svelte#155 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

would that work with yarn? also the search dir setting is likely needed if the users add more prettier plugins.

Copy link
Member

@dummdidumm dummdidumm Sep 19, 2022

Choose a reason for hiding this comment

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

Just from reading this I'm not sure if the requested change by @bluwy is the same as the proposal, or works for cases where the proposed changes by @dominikg don't work?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm yeah my suggestion doesn't seem to work with yarn pnp (pnp also messes up vscode prettier plugin with this PR change and before). I guess we can go with this PR change still to have it partially working for yarn pnp.

also the search dir setting is likely needed if the users add more prettier plugins.

Since they'd have to still add them in plugins with this PR, e.g. "plugins": ["prettier-plugin-svelte", "prettier-plugin-something"], I don't think the plugin-search-dir would make a difference, besides not needing the ./node_modules/ part of my suggestion.

Copy link
Member Author

Choose a reason for hiding this comment

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

i prefer not having node_modules listed explicitly. I'd love to get rid of the --plugin-search-dir . on the cli though so it's a tough one. Ultimately this PR uses multiple ways to try and ensure the plugin is found. The only other option i know would be to use prettierrc.cjs and require('prettier-plugin-svelte') to take resolving out of prettiers own hands. But thats also ugly.

I think the way this PR uses both json rc and cli option to ensure the plugin is found during api and cli use is working and has only one duplication, which is unfortunate but not that bad.

Copy link
Member

Choose a reason for hiding this comment

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

Let's go with this solution then

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.

Prettier is ignoring svelte files
3 participants