feat: support handling `.svelte` files by robertheessels · Pull Request #950 · prettier/prettier-eslint

robertheessels · 2024-01-18T15:22:43Z

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

close #525

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

changeset-bot · 2024-01-18T15:22:47Z

🦋 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 · 2024-01-18T15:49:41Z

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

robertheessels · 2024-01-19T09:55:32Z

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

JounQin · 2024-01-19T09:58:15Z

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

As I don't have any experience with eslint complexity rule, I'm not 100% sure this is the correct way to do it.

JounQin · 2024-01-19T10:54:13Z

@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 · 2024-01-19T11:49:43Z

@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 · 2024-01-19T11:53:41Z

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 · 2024-01-19T15:07:47Z

@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 · 2024-01-19T15:27:00Z

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 · 2024-01-19T16:06:01Z

@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 · 2024-01-19T16:44:25Z

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 · 2024-01-19T16:45:00Z

@robertheessels

I believe it should be added as an optional dependency.

I have no idea how to do that?

peerDependencies

peerDependenciesMeta

robertheessels · 2024-01-20T09:02:52Z

@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 · 2024-01-20T09:08:39Z

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

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

robertheessels · 2024-01-20T09:23:02Z

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 · 2024-01-20T09:30:04Z

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

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

svelte package needed for test.

package.json

JounQin · 2024-01-20T15:38:00Z

@robertheessels Thanks for your contribution!

robertheessels · 2024-01-20T15:48:12Z

@JounQin You're welcome!

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

JounQin · 2024-01-20T16:13:59Z

@robertheessels It's already been listed in devDependencies.

robertheessels · 2024-01-20T16:46:28Z

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 · 2024-01-20T17:06:46Z

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

Added Svelte handling.

75830e3

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

robertheessels mentioned this pull request Jan 18, 2024

Can you support Svelte? #525

Closed

robertheessels added 2 commits January 19, 2024 11:29

Added eslint complexity disable.

f59e4a1

As I don't have any experience with eslint complexity rule, I'm not 100% sure this is the correct way to do it.

Added svelte test.

3bdaad4

Fixed Svelte test.

cc48d40

Make Svelte packages optional.

9a4f132

Using peerDependencies.

597d9b9

svelte package needed for test.

JounQin reviewed Jan 20, 2024

View reviewed changes

package.json Outdated Show resolved Hide resolved

JounQin reviewed Jan 20, 2024

View reviewed changes

package.json Outdated Show resolved Hide resolved

JounQin reviewed Jan 20, 2024

View reviewed changes

package.json Outdated Show resolved Hide resolved

robertheessels and others added 3 commits January 20, 2024 16:11

Better versions.

a11892e

Update package.json

434ae88

Create fair-foxes-brake.md

1791e37

JounQin changed the title ~~Added Svelte handling.~~ feat: support handling .svelte files Jan 20, 2024

JounQin approved these changes Jan 20, 2024

View reviewed changes

JounQin merged commit 8418438 into prettier:master Jan 20, 2024

github-actions bot mentioned this pull request Jan 20, 2024

chore: release prettier-eslint #951

Merged

Uh oh!

Conversation

robertheessels commented Jan 18, 2024 • edited by JounQin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot bot commented Jan 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

JounQin commented Jan 18, 2024

Uh oh!

robertheessels commented Jan 19, 2024

Uh oh!

JounQin commented Jan 19, 2024

Uh oh!

JounQin commented Jan 19, 2024

Uh oh!

robertheessels commented Jan 19, 2024

Uh oh!

JounQin commented Jan 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robertheessels commented Jan 19, 2024

Uh oh!

JounQin commented Jan 19, 2024

Uh oh!

robertheessels commented Jan 19, 2024

Uh oh!

codecov-commenter commented Jan 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

JounQin commented Jan 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robertheessels commented Jan 20, 2024

Uh oh!

JounQin commented Jan 20, 2024

Uh oh!

robertheessels commented Jan 20, 2024

Uh oh!

JounQin commented Jan 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JounQin commented Jan 20, 2024

Uh oh!

robertheessels commented Jan 20, 2024

Uh oh!

JounQin commented Jan 20, 2024

Uh oh!

robertheessels commented Jan 20, 2024

Uh oh!

JounQin commented Jan 20, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

robertheessels commented Jan 18, 2024 •

edited by JounQin

Loading

changeset-bot bot commented Jan 18, 2024 •

edited

Loading

JounQin commented Jan 19, 2024 •

edited

Loading

codecov-commenter commented Jan 19, 2024 •

edited

Loading

JounQin commented Jan 19, 2024 •

edited

Loading

JounQin commented Jan 20, 2024 •

edited

Loading