feat: support svelte(prettier-plugin-svelte) and angular parser#156
feat: support svelte(prettier-plugin-svelte) and angular parser#156JounQin wants to merge 1 commit intoprettier:masterfrom rx-ts:patch-1
svelte(prettier-plugin-svelte) and angular parser#156Conversation
JounQin
changed the title
prettier-plugin-svelteprettier-plugin-svelte, fix test cases - close #157
JounQin
changed the title
prettier-plugin-svelte, fix test cases - close #157svelte(prettier-plugin-svelte) and angular parser, fix test cases
JounQin
changed the title
svelte(prettier-plugin-svelte) and angular parser, fix test casessvelte(prettier-plugin-svelte) and angular parser
| describe('E2E Tests', () => { | ||
| test('CSS/SCSS files', () => { | ||
| const result = runStylelint('*.{css,scss}'); | ||
| test('CSS files', () => { |
There was a problem hiding this comment.
I don't know why, but the order of the result are reversed sometimes, so I split the two type of files.
There was a problem hiding this comment.
Huh, that is annoying. I've addressed this separately so this PR can focus on the svelte change: #159
Rebase atop latest master and you should be able to remove this.
There was a problem hiding this comment.
Thanks for the PR!
It does a couple of things unrelated to svelte/angular support, and I've pulled out avoiding flaky tests and testing against prettier v2 into separate PRs.
In light of those changes to help you focus this PR, can you:
- Rebase atop latest master
- Remove the flaky tests fixes as they're already in master
- Remove the prettier v2 ci and the "endOfLine" changes to get that working, as they're already in master (but with a different approach, you shouldn't need to add endOfLine into any prettierrc files)
- Check if renaming the "angular" test fixture file to
check.inert.component.htmlwill trigger the angular parser and if so, remove the override for "angular.html" files from .prettierrc
| @@ -1,4 +1,3 @@ | |||
| package.json | |||
There was a problem hiding this comment.
Why does this need to be removed? I would expect that the format of package.json is automatically handled by yarn/npm rather than being something that prettier needs to be concerned about
| "trailingComma": "es5", | ||
| "bracketSpacing": false | ||
| "bracketSpacing": false, | ||
| "endOfLine": "auto", |
There was a problem hiding this comment.
This can be removed, thanks to the more targeted fix in #158
| jobs: | ||
| ci: | ||
| name: 'Test: Node ${{ matrix.node-version }} - Stylelint ${{ matrix.stylelint-version }}' | ||
| name: 'Test: Node ${{ matrix.node-version }} - Prettier ${{ matrix.prettier-version }} - Stylelint ${{ matrix.stylelint-version }}' |
There was a problem hiding this comment.
I've split this out into a separate commit in #158, where I add Prettier to the test matrix and perform a more targeted fix for the "endOfLine" stuff.
Rebase and you should be able to remove the changes to this file, and all instances where you add "endOfLine": "auto" to configs.
| expect(result.status).toEqual(0); | ||
| }); | ||
|
|
||
| test('Svelte files', () => { |
There was a problem hiding this comment.
Svelte files get treated the same as HTML/Markdown/Vue files in that they are inert. Rather than add a new test case just for Svelte, let's add them to the existing "HTML/Markdown/Vue files" test case by updating the name and changing like 23 to be const result = runStylelint('*.{html,md,vue,svelte}');.
| describe('E2E Tests', () => { | ||
| test('CSS/SCSS files', () => { | ||
| const result = runStylelint('*.{css,scss}'); | ||
| test('CSS files', () => { |
There was a problem hiding this comment.
Huh, that is annoying. I've addressed this separately so this PR can focus on the svelte change: #159
Rebase atop latest master and you should be able to remove this.
| "tabWidth": 4, | ||
| "trailingComma": "all" | ||
| "trailingComma": "all", | ||
| "endOfLine": "auto" |
There was a problem hiding this comment.
This can be removed, thanks to the more targeted fix in #158
| "singleQuote": true, | ||
| "tabWidth": 2, | ||
| "trailingComma": "es5", | ||
| "endOfLine": "auto", |
There was a problem hiding this comment.
This can be removed, thanks to the more targeted fix in #158
| @@ -0,0 +1,16 @@ | |||
| <!DOCTYPE html> | |||
There was a problem hiding this comment.
https://github.com/prettier/prettier/blob/ce26805e84756953875f30a0c165bd6b09c94ea0/src/language-html/index.js#L13 suggests that you can trigger the angular parser by using the .component.html extension.
If we name this file check.inert.component.html then it looks like we should trigger using the angular component without needing to add any of the overrides in our base prettierrc.
| "bracketSpacing": false | ||
| "bracketSpacing": false, | ||
| "endOfLine": "auto", | ||
| "overrides": [ |
There was a problem hiding this comment.
By naming our fixture file check.inert.component.html we should be able to trigger the angular parser without needing to add this custom override. Can you confirm if that works and if so rename the fixture file and remove this?
|
As I was feeling on a roll I've addressed my concerns in #160 rather than making you do it all. Thanks for addressing this in the first place though, your investigation made my work a lot easier! I've published stylelint-prettier 1.2.0 with support for the angular and svelte parsers. I'm going to close this PR as I think the root issue has been addressed. If you're still having issues please reopen with a set of changes against latest master. |
2 participants
sveltejs/prettier-plugin-svelte#210
close #25
@BPScott Please help to review.