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 prettier #16

Open
IlyaSemenov opened this issue May 23, 2019 · 17 comments
Open

Support prettier #16

IlyaSemenov opened this issue May 23, 2019 · 17 comments
Labels
enhancement New feature or request

Comments

@IlyaSemenov
Copy link

I know that the plugin doesn't work with prettier at the moment. However, the documentation doesn't explain the rationale behind this decision, and whether you have interest in "fixing" it or not. I suggest to either amend the documentation, or leave this issue opened as a part of the road map.

For example, eslint-plugin-vue works just fine with prettier, and I don't see how Vue components are logically different from Svelte components.

@Conduitry
Copy link
Member

Adding this hasn't been ruled out. A while ago I did actually fiddle with this some in this outdated branch, and it seemed to work reasonably well. It's mostly a matter of carefully adding comments to disable and reenable Prettier as well as for ESLint.

@Conduitry Conduitry added the enhancement New feature or request label May 24, 2019
@iztsv
Copy link

iztsv commented Jul 29, 2019

Seems, it does not work.

I have the following .eslintrc.js:

module.exports = {
  extends: ['prettier'],
  parserOptions: {
    ecmaVersion: 2019,
    sourceType: 'module'
  },
  env: {
    es6: true,
    browser: true
  },
  plugins: [
    'prettier',
    'svelte3'
  ],
  overrides: [
    {
      files: ['**/*.svelte'],
      processor: 'svelte3/svelte3',
      settings: {
        'svelte3/ignore-styles': attrs => attrs.type === 'text/postcss',
        'svelte3/lint-template': false
      },
    }
  ],
  rules: {
    'prettier/prettier': 'error'
  }
};

and that component:

<div class="datetime">
  <span class="item _time">{ data.time }</span>
  <span class="item">{ data.date }</span>
</div>

after eslint --ext js,svelte ., I've got:

<div class="datetime">
  <span class="item _time">{ data.time }</span>
  <span class="item">{ data.date; }</span>
</div>

during npm run dev (sapper) I get the following error:

✗ client
Failed to build — error in src/routes/_layout/header/DateTime.svelte: Expected }
1: <div class="datetime">
2:   <span class="item _time">{ data.time }</span>
3:   <span class="item">{ data.date; }</span>
                                   ^
4: </div>

@dummdidumm
Copy link
Member

As stated in the newest Prettier docs, using packages like eslint-plugin-prettier is no longer recommended. Instead, use a preset configuration which turns off all Eslint rules which conflict with prettier and then just format first with prettier and run the linter afterwards, making it a two-step-process which is nicely decoupled.

@mattfysh
Copy link

mattfysh commented Feb 22, 2021

After some digging, this appears to be caused by the eslint-plugin-svelte3 processor which runs on *.svelte files and creates chunks of code that are then piped through eslint. If you've also enabled eslint-plugin-prettier -- and presumable prettier-plugin-svelte -- it will receive the processed chunks and try to push them through the svelte compiler, even though that has already been done by the processor. This is what is causing the svelte compile error

I was hoping to get this to work for GitHub Actions, so that all linting and styling errors are both reported through the same mechanism, so that they can then be written as inline errors into the Pull Request UI

If anyone knows of another way to use prettier --check and have the errors reported back into GitHub pull requests in the same way, I'm all 👂 s

@dummdidumm
Copy link
Member

What's the problem with chaining this in one npm script? "lint": "eslint ..(config stuff) && prettier --check (..stuff)"

@mattfysh
Copy link

mattfysh commented Feb 22, 2021

@dummdidumm I'm looking to make use of this Github linting action: https://github.com/wearerequired/lint-action ... so that any errors picked up by eslint are visible in the Pull Request UI

I believe that if I can get eslint-plugin-prettier working, I'd be able to expose styling/formatting errors in the pull request UI also

The issue is that there's no compatibility between eslint, prettier, and svelte source code.

The other solution is to remove eslint-plugin-prettier and run a separate prettier --check task as suggested, I'd just need to figure out how to get the errors found by the prettier CLI to bubble up into the Pull Request UI

@JounQin
Copy link

JounQin commented Apr 30, 2021

Maybe prettier/eslint-plugin-prettier#415 this PR would help, I can add svelte be to ignored in favor of prettier-plugin-svelte.

@dummdidumm
Copy link
Member

I don't understand this fully. Does this mean that eslint-plugin-prettier would ignore formatting the virtual .js files eslint-plugin-svelte3 generates and instead run the prettier check on the whole .svelte file? If so I think this would solve the interop issue and maybe also fix sveltejs/prettier-plugin-svelte#57

@JounQin
Copy link

JounQin commented Apr 30, 2021

@dummdidumm

Yes! But *.svelte must be parsable by ESLint, so that the whole file can be run by eslint-plugin-prettier, not code blocks by processor. But it seems not possible by eslint-plugin-svelte3 currently, svelte-eslint-parser may help.

Or we can make a fake parser for .svelte to make sure it won't crash.

Then in processor we do:

{
  preprocess(text, filename) {
    return [
      text, // represents the whole `.svelte` file, will be parsed by fake parser and be compatible with `eslint-plugin-prettier`
      // .. original code blocks
    ]
  }
}
// fake parser
exports.parseForESLint = function() {
  return {
    // ...
    ast: {
      // ...
    }
  }
}

Final usage

{
  "parser": "eslint-plugin-svelte3",
  "prosessor": "eslint-plugin-svelte3"
}

Or, we should just ignore this isse. User should always use prettier --write *.svelte

@dummdidumm
Copy link
Member

Now I'm confused. I thought that eslint-plugin-prettier will look at whatever is handed to it, run prettier on it, and report back the differences as eslint warnings. If the generated .js files by eslint-plugin-svelte3 would be ignored and instead the whole .svelte file is formatted, prettier-plugin-svelte should step in and format this. Where in this chain does ESLint need to know how to parse .svelte files?

@JounQin
Copy link

JounQin commented Apr 30, 2021

@dummdidumm

If the generated .js files by eslint-plugin-svelte3 would be ignored and instead the whole .svelte file is formatted, prettier-plugin-svelte should step in and format this

A rule only cares about text and filename, and code blocks can be detected.

In my PR prettier/eslint-plugin-prettier#415 (or similar prettier/stylelint-prettier#22), if the whole file can be parsed by prettier according to the file extension, eslint-plugin-prettier will just return and ignore the code block because it is redundant.

But the question is how could the whole file be run with eslint-plugin-prettier again.

The answer is the fake parser and text in preprocess.

@JounQin
Copy link

JounQin commented Apr 30, 2021

You can see this pattern at the following:

  1. fake parser: https://github.com/rx-ts/eslint/blob/main/packages/htm/src/parser.ts
  2. A custom rule which handles html file(not script inside): https://github.com/rx-ts/eslint/blob/main/packages/htm/src/rules/html.ts

@BPScott
Copy link

BPScott commented May 25, 2021

Hi, eslint-plugin-prettier maintainer here :)

Interop between eslint-plugin-prettier and processors that teach eslint about non-js file types gets FUN, however recently @JounQin shown me the concept of processors providing virtual filenames, which reduces potential for wonky interop.

Currently eslint-plugin-svelte3 doesn't leverage these virtual filenames by default - only if named_blocks is set.

This means that when named_blocks is set - eslint-plugin-prettier sees the virtual filename BLAH/foo.svelte/module.js and says "prettier should try to parse the code returned by the processor as JS, thanks to the .js extension on the virtual filename" and all is good in the world as the code is indeed JS.

However when named_blocks is not set there is no virtual filename, only the "on disk" filename BLAH/foo.svelte and thus eslint-plugin-pretier says "I will try to parse that code using the svelte parser thanks to the .svelte extension" and then promptly explodes because the code that it tries to process is not a svelte file at all but a blob of JS extracted from the original svelte file.

Now eslint-prettier-plugin does maintain a list of cases where a processor does not use virtual filenames and thus it should force using the JS parser instead of whatever gets inferred from the filename, but it'd be super nice if I didn't have to add support for every processor.

It's a clunky breaking change on your end, (which sucks, sorry) but I think the best long-term fix might be to remove the named_blocks setting and have it's behaviour on by default for the sake of better interoperability.

This will also improve interop with other situations such as when a user only enables some eslint typescript-specific rules for *.ts - by adding virtual filenames these rules will be ran over TS in svelte files when the typescript option is enabled

@kazzkiq
Copy link

kazzkiq commented Oct 4, 2021

While is clearly not the path @BPScott want to follow, I can confirm that updating the array in this line by adding "svelte" to it makes things work.

However after that Prettier starts to complain about some rules inside JavaScript expressions, which also ends up leading to new errors. Example:

<h1>Hello, {name}</h1>

Prettier forces the above code to:

<h1>Hello, {name;}</h1>

This kind of rules should only be applied to JavaScript inside <script> tags, not expressions ({}).

So making Prettier work properly with ESLint in a Svelte project will be probably a little harder and maybe require some fine tuning in one or more related plugins.

Edit: I've setup a minimal repo to help reproduce the errors when trying to use ESLint, Prettier and Svelte:

https://github.com/kazzkiq/svelte-eslint-prettier-error

@dummdidumm
Copy link
Member

dummdidumm commented Jan 17, 2022

The proposed solution in #16 (comment) is probably not feasible because TS users have to switch it back because apparently the TS parser doesn't like virtual files: #162

@JounQin
Copy link

JounQin commented Jun 30, 2022

@o-az
Copy link

o-az commented Jul 3, 2022

thank you @JounQin for the info. I've found eslint-plugin-svelte much more useful.

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

No branches or pull requests

9 participants