feat: support handling .svelte files

[robertheessels]

Without this addition, prettier-eslint will only run the Prettier step for .svelte files, but NOT the Eslint step!

close #525

[changeset-bot]

🦋 Changeset detected

Latest commit: 1791e37

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

This PR includes changesets to release 1 package

Name	Type
prettier-eslint	Minor

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

[JounQin]

Codes look good to me, CI is broken, maybe a eslint-disable is required.

[robertheessels]

I'm new to this repo. Does the lint problem have anything to do with my pull request at all?

[JounQin]

I'm new to this repo. Does the lint problem have anything to do with my pull request at all?

The CI is broken because the new codes add one more complexity. See

[lint] /home/runner/work/prettier-eslint/prettier-eslint/src/index.js
[lint]   59:1  warning  Async function 'analyze' has a complexity of 14. Maximum allowed is 13  complexity
[lint] 
[lint] ✖ 1 problem (0 errors, 1 warning)

https://github.com/prettier/prettier-eslint/actions/runs/7572175398/job/20622456917?pr=950#step:5:17

[JounQin]

@robertheessels Test case is missing, you can follow

prettier-eslint/src/__tests__/index.js

Lines 182 to 195 in 689f67a

	{
	title: 'Vue.js example',
	input: {
	eslintConfig: {
	rules: {
	'space-before-function-paren': [2, 'always']
	}
	},
	text: '<template>\n <div></div>\n</template>\n<script>\nfunction foo() { return "foo" }\n</script>\n<style>\n</style>',
	filePath: path.resolve('./test.vue')
	},
	output:
	'<template>\n <div></div>\n</template>\n<script>\nfunction foo () {\n return "foo";\n}\n</script>\n<style></style>'
	},

to add a new svelte test case.

[robertheessels]

@JounQin I added the test an hour ago (even before your comment 😄), but the ci still seems to be in queue or something?
ci (18) Expected — Waiting for status to be reported

[JounQin]

https://github.com/prettier/prettier-eslint/actions/runs/7582808184/job/20654893335?pr=950

@robertheessels

You'll need to add https://github.com/sveltejs/prettier-plugin-svelte at https://github.com/prettier/prettier-eslint/blob/master/.prettierrc.js

Could you please run the test cases successfully before committing?

I believe it should be added as an optional dependency.

[robertheessels]

@JounQin Sorry for not running test locally, doing so now.

Is .prettierrc.js even used in the tests? I have tried adding prettier-plugin-svelte to it, as I have done successfully in my other project, and in a few other different ways, but always it errors with No parser could be inferred.

Even if I add invalid strings in .prettierrc.js, the tests run without complaining about that, so I was wondering if .prettierrc.js is used in the tests?

[JounQin]

It should be related to prettier.resolveConfig is mocked on testing at

prettier-eslint/src/__mocks__/prettier.js

Line 10 in 689f67a

resolveConfig: jest.fn()

You can change it just like mockFormat to be a wrapper

[robertheessels]

@JounQin All tests passing locally. I had to add more packages than I expected, like svelte itself. Not sure if that is desirable or acceptable. But this way prettier-eslint can now handle Svelte files.

I believe it should be added as an optional dependency.

I have no idea how to do that?

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (689f67a) 100.00% compared to head (1791e37) 100.00%.

Additional details and impacted files

@@            Coverage Diff            @@
##            master      #950   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines          301       303    +2     
  Branches        83        84    +1     
=========================================
+ Hits           301       303    +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[JounQin]

@robertheessels

I believe it should be added as an optional dependency.

I have no idea how to do that?

peerDependencies

peerDependenciesMeta

[robertheessels]

@JounQin

If I put any of my 3 packages in peerDependencies, the test will fail, as npm i will not install them.
I have now put the in optionalDependencies, which works, as npm will now install them.
Or should we change the npm i in ci.yml?

I have no experience at all with peerDependencies and optionalDependencies, so I really don't know what is best here?

[JounQin]

It's peerDependencies and peerDependenciesMeta, not optionalDependencies, as I linked above.

I'll edit this PR later when I'm free then.

[robertheessels]

It's peerDependencies and peerDependenciesMeta, not optionalDependencies, as I linked above.

I'll edit this PR later when I'm free then.

See my last comment. :-)

[JounQin]

I don't understand, you were saying optionalDependencies, I'm saying peerDependenciesMeta?

Both peerDependencies and devDependencies should be used. (svelte should not be peer.)

[JounQin]

@robertheessels Thanks for your contribution!

[robertheessels]

@JounQin You're welcome!

Did the tests run in CI without the svelte package? They failed for me locally without it.

[JounQin]

@robertheessels It's already been listed in devDependencies.

[robertheessels]

Right.

Once a new prettier-eslint version is released, I can update the docs, if you want. We should tell users about the Svelte option? And maybe also suggested Prettier and Eslint configs for Svelte?

[JounQin]

Oh, sure, I totally forgot about the document part, feel free to raise a new PR.