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

feat: support svelte(prettier-plugin-svelte) and angular parser #156

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,18 @@ on:

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 }}'
Copy link
Member

Choose a reason for hiding this comment

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

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.

runs-on: ubuntu-latest
strategy:
matrix:
prettier-version: [1.x, 2.x]
stylelint-version: [13.x, 12.x, 11.x, 10.x, 9.5.x]
node-version: [14.x, 12.x, 10.x, 8.x]

exclude:
# prettier 2 does not support node 8
- prettier-version: 2.x
node-version: 8.x
# stylelint 13 does not support node 8
- stylelint-version: 13.x
node-version: 8.x
Expand All @@ -27,6 +31,9 @@ jobs:
with:
node-version: ${{ matrix.node-version }}

- name: Use Prettier ${{ matrix.prettier-version }}
run: yarn upgrade prettier@${{ matrix.prettier-version }}

- name: Use Stylelint ${{ matrix.stylelint-version }}
run: yarn upgrade stylelint@${{ matrix.stylelint-version }}

Expand Down
1 change: 0 additions & 1 deletion .prettierignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
package.json
Copy link
Member

Choose a reason for hiding this comment

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

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

test/fixtures

# this file doesn't exist, but we use it as a filename that should be ignored
Expand Down
11 changes: 10 additions & 1 deletion .prettierrc
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,14 @@
"arrowParens": "always",
"singleQuote": true,
"trailingComma": "es5",
"bracketSpacing": false
"bracketSpacing": false,
"endOfLine": "auto",
Copy link
Member

Choose a reason for hiding this comment

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

This can be removed, thanks to the more targeted fix in #158

"overrides": [
Copy link
Member

Choose a reason for hiding this comment

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

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?

{
"files": "*.angular.html",
"options": {
"parser": "angular"
}
}
]
}
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,11 @@
"eslint-plugin-prettier": "^3.1.1",
"jest": "^24.9.0",
"prettier": "^1.19.1",
"prettier-plugin-svelte": "^2.1.6",
"strip-ansi": "^6.0.0",
"stylelint": "^9.5.0",
"stylelint-config-prettier": "^7.0.0"
"stylelint-config-prettier": "^7.0.0",
"svelte": "^3.32.3"
},
"engines": {
"node": ">=6"
Expand Down
8 changes: 5 additions & 3 deletions stylelint-prettier.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ module.exports = stylelint.createPlugin(
initialOptions.parser = 'css';
}

// Stylelint suppports languages that may contain multiple types of style
// Stylelint supports languages that may contain multiple types of style
// languages, thus we can't rely on guessing the parser based off the
// filename.

Expand All @@ -94,6 +94,8 @@ module.exports = stylelint.createPlugin(
'vue',
'markdown',
'html',
'angular',
'svelte', // https://github.com/sveltejs/prettier-plugin-svelte/issues/210
];
if (parserBlockList.indexOf(prettierFileInfo.inferredParser) !== -1) {
return;
Expand All @@ -116,7 +118,7 @@ module.exports = stylelint.createPlugin(
let message = 'Parsing error: ' + err.message;

// Prettier's message contains a codeframe style preview of the
// invalid code and the line/column at which the error occured.
// invalid code and the line/column at which the error occurred.
// ESLint shows those pieces of information elsewhere already so
// remove them from the message
if (err.codeFrame) {
Expand Down Expand Up @@ -190,7 +192,7 @@ module.exports = stylelint.createPlugin(
const syntax = root.source.syntax || result.opts.syntax;
const newRoot = syntax.parse(rawData);

// For reasons I don't really undersand, when the original input does
// For reasons I don't really understand, when the original input does
// not have a trailing newline, newRoot generates a trailing newline but
// it does not get included in the output.
// Cleaning the root raws (to remove any existing whitespace), then
Expand Down
16 changes: 16 additions & 0 deletions test/fixtures/check.angular.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<!DOCTYPE html>
Copy link
Member

Choose a reason for hiding this comment

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

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.

<html>
<head>
<meta charset="utf-8" />
<title>Page Title</title>
<style>
.foo {
background-image: url('x');
}
</style>
</head>

<body>
<p class="mb-0" style="font-size: 12px; background-color: red">Hi</p>
</body>
</html>
13 changes: 13 additions & 0 deletions test/fixtures/check.inert.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<span>Hi</span>

<style lang="scss">
.foo {
background-image: url("x")
}

$map: (
'alpha': 10,
'beta': 20,
'gamma': 30
)
</style>
3 changes: 2 additions & 1 deletion test/prettierrc/custom/.prettierrc
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"singleQuote": false,
"tabWidth": 4,
"trailingComma": "all"
"trailingComma": "all",
"endOfLine": "auto"
Copy link
Member

Choose a reason for hiding this comment

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

This can be removed, thanks to the more targeted fix in #158

}
1 change: 1 addition & 0 deletions test/prettierrc/default/.prettierrc
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"singleQuote": true,
"tabWidth": 2,
"trailingComma": "es5",
"endOfLine": "auto",
Copy link
Member

Choose a reason for hiding this comment

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

This can be removed, thanks to the more targeted fix in #158

"overrides": [
{
"files": "*.wxss",
Expand Down
22 changes: 20 additions & 2 deletions test/stylelint-prettier-e2e.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,22 @@ const {resolve} = require('path');
const stripAnsi = require('strip-ansi');

describe('E2E Tests', () => {
test('CSS/SCSS files', () => {
const result = runStylelint('*.{css,scss}');
test('CSS files', () => {
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 don't know why, but the order of the result are reversed sometimes, so I split the two type of files.

https://github.com/prettier/stylelint-prettier/runs/1954057143?check_suite_focus=true#step:7:9

Copy link
Member

Choose a reason for hiding this comment

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

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.

const result = runStylelint('*.css');

const expectedResult = `
check.invalid.css
2:25 ✖ Replace ""x"" with "'x'" prettier/prettier
`.trim();

expect(result.output).toEqual(expectedResult);
expect(result.status).toEqual(2);
});

test('SCSS files', () => {
const result = runStylelint('*.scss');

const expectedResult = `
check.invalid.scss
2:25 ✖ Replace ""x"" with "'x'" prettier/prettier
8:14 ✖ Insert "," prettier/prettier
Expand Down Expand Up @@ -39,6 +48,15 @@ check.invalid.scss
expect(result.output).toEqual(expectedResult);
expect(result.status).toEqual(0);
});

test('Svelte files', () => {
Copy link
Member

Choose a reason for hiding this comment

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

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}');.

const result = runStylelint('*.svelte');

const expectedResult = ``;

expect(result.output).toEqual(expectedResult);
expect(result.status).toEqual(0);
});
});

function runStylelint(pattern) {
Expand Down
10 changes: 10 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3561,6 +3561,11 @@ prettier-linter-helpers@^1.0.0:
dependencies:
fast-diff "^1.1.2"

prettier-plugin-svelte@^2.1.6:
version "2.1.6"
resolved "https://registry.yarnpkg.com/prettier-plugin-svelte/-/prettier-plugin-svelte-2.1.6.tgz#ae10e16d89c94c72a937574eb840501104ff4565"
integrity sha512-eg6MhH394IYsljIcLMgCx/wozObkUFZJ1GkH+Nj8sZQilnNCFTeSBcEohBaNa9hr5RExvlJQJ8a2CEjMMrwL8g==

prettier@^1.19.1:
version "1.19.1"
resolved "https://registry.yarnpkg.com/prettier/-/prettier-1.19.1.tgz#f7d7f5ff8a9cd872a7be4ca142095956a60797cb"
Expand Down Expand Up @@ -4333,6 +4338,11 @@ supports-color@^6.1.0:
dependencies:
has-flag "^3.0.0"

svelte@^3.32.3:
version "3.32.3"
resolved "https://registry.yarnpkg.com/svelte/-/svelte-3.32.3.tgz#db0c50c65573ecffe4e2f4924e4862d8f9feda74"
integrity sha512-5etu/wDwtewhnYO/631KKTjSmFrKohFLWNm1sWErVHXqGZ8eJLqrW0qivDSyYTcN8GbUqsR4LkIhftNFsjNehg==

svg-tags@^1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/svg-tags/-/svg-tags-1.0.0.tgz#58f71cee3bd519b59d4b2a843b6c7de64ac04764"
Expand Down