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

Added prettier-plugin for formatting glimmer component templates #117

Merged
merged 9 commits into from
Jul 15, 2021
Merged
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
2 changes: 2 additions & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,5 @@
**/packages/@glimmerx/babel-preset/test/**/output.js
**/packages/@glimmerx/babel-preset/test/**/code.ts
**/packages/@glimmerx/babel-preset/test/**/output.ts
**/packages/@glimmerx/prettier-plugin-component-templates/test/fixtures/**/code.*
**/packages/@glimmerx/prettier-plugin-component-templates/test/fixtures/**/output.*
7 changes: 3 additions & 4 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ module.exports = {
env: {
es6: true,
},
extends: ['eslint:recommended', 'plugin:prettier/recommended'],
plugins: ['@typescript-eslint', '@glimmerx', 'prettier'],
extends: ['eslint:recommended'],
plugins: ['@typescript-eslint', '@glimmerx'],
rules: {
'@glimmerx/template-vars': 'error',
},
Expand Down Expand Up @@ -42,6 +42,7 @@ module.exports = {
files: [
'packages/@glimmerx/babel-preset/**/*.js',
'packages/@glimmerx/eslint-plugin/**/*.js',
'packages/@glimmerx/prettier-plugin-component-templates/**/*.js',
'packages/@glimmerx/webpack-loader/**/*.js',
],
env: {
Expand All @@ -63,11 +64,9 @@ module.exports = {
extends: [
'plugin:@typescript-eslint/eslint-recommended',
'plugin:@typescript-eslint/recommended',
'prettier/@typescript-eslint',
],
rules: {
'@typescript-eslint/no-explicit-any': 'error',
'@typescript-eslint/explicit-function-return-type': 'error',
'@typescript-eslint/no-unused-vars': ['error', { argsIgnorePattern: '^_' }],

'@typescript-eslint/ban-types': [
Expand Down
3 changes: 3 additions & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
@@ -1 +1,4 @@
packages/@glimmerx/babel-preset/test/
packages/@glimmerx/prettier-plugin-component-templates/test/fixtures/*/code.*
packages/@glimmerx/prettier-plugin-component-templates/test/fixtures/*/output.*

3 changes: 2 additions & 1 deletion .prettierrc
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"singleQuote": true,
"trailingComma": "es5",
"printWidth": 100
"printWidth": 100,
"plugins": ["./packages/@glimmerx/prettier-plugin-component-templates"]
}
10 changes: 5 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,17 @@
],
"scripts": {
"build": "scripts/build.js",
"format:check": "prettier --check 'packages/**/*.{js,ts}'",
Copy link
Collaborator

Choose a reason for hiding this comment

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

shouldn't this just be done by the prettier plugin in eslint?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When prettier and eslint both do formatting we get sometimes a double rewrite of a file, with the collision of rules being applied to a tagged template expression.

What we're doing now is simply using prettier to do a style-check format and do a check then use eslint lint for anti-patterns.

"format": "prettier --write 'packages/**/*.{js,ts}'",
"lint": "eslint . --cache --ext .js,.ts",
"prepublishOnly": "scripts/build.js",
"problems": "tsc -p tsconfig.json --noEmit",
"start": "webpack-dev-server",
"storybook": "yarn build && yarn workspace basic-example-app storybook",
"test": "yarn lint && testem ci && yarn test:babel-plugins",
"test": "yarn format:check && yarn lint && testem ci && yarn test:babel-plugins && yarn test:prettier",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We may not want this here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I respond to this in a comment by @chiragpat below.

"test:babel-plugins": "mocha -r esm --timeout 5000 packages/@glimmerx/babel-preset/test packages/@glimmerx/eslint-plugin/test/lib/rules",
"test:ember": "yarn workspace basic-addon ember try:one",
"test:prettier": "mocha -r esm packages/@glimmerx/prettier-plugin-component-templates/test",
"test:playground": "yarn workspace glimmerx-playground test",
"test:watch": "testem"
},
Expand All @@ -48,12 +50,10 @@
"@typescript-eslint/eslint-plugin": "^4.18.0",
"@typescript-eslint/parser": "^4.18.0",
"babel-loader": "^8.0.6",
"eslint": "^6.8.0",
"eslint-config-prettier": "^6.10.1",
"eslint-plugin-prettier": "^3.1.2",
"eslint": "^7.29.0",
"fs-extra": "^9.0.0",
"lerna": "^3.20.2",
"prettier": "^2.0.4",
"prettier": "^2.3.1",
"qunit": "^2.9.3",
"release-it": "^13.5.1",
"release-it-lerna-changelog": "^2.1.2",
Expand Down
15 changes: 5 additions & 10 deletions packages/@glimmerx/blueprint/files/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,12 @@
"clean-webpack-plugin": "^3.0.0",
"copy-webpack-plugin": "^5.1.1",
"css-loader": "^3.4.2",
"eslint": "^6.8.0",
"eslint-config-prettier": "^6.10.1",
"eslint-plugin-prettier": "^3.1.2",
"eslint": "^7.29.0",
"file-loader": "^6.0.0",
"glob": "7.1.6",
"html-webpack-plugin": "^4.0.4",
"npm-run-all": "^4.1.5",
"prettier": "^2.0.2",
"prettier": "^2.3.1",
"qunit": "^2.9.3",
"qunit-dom": "^1.1.0",
"style-loader": "^1.1.3",
Expand All @@ -61,12 +59,10 @@
},
"plugins": [
"@glimmerx",
"@typescript-eslint",
"prettier"
"@typescript-eslint"
],
"extends": [
"eslint:recommended",
"plugin:prettier/recommended"
"eslint:recommended"
],
"ignorePatterns": [
"dist/",
Expand All @@ -83,8 +79,7 @@
],
"extends": [
"plugin:@typescript-eslint/eslint-recommended",
"plugin:@typescript-eslint/recommended",
"prettier/@typescript-eslint"
"plugin:@typescript-eslint/recommended"
]
},
{
Expand Down
28 changes: 14 additions & 14 deletions packages/@glimmerx/core/tests/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,19 @@ import renderTests, { Constructor } from './render-tests';
import { renderComponent, RenderComponentOptions } from '..';
import Component from '@glimmerx/component';

renderTests(
'@glimmerx/core',
async (component: Constructor<Component>, options?: RenderComponentOptions) => {
const element = document.getElementById('qunit-fixture')!;
element.innerHTML = '';
renderTests('@glimmerx/core', async (
component: Constructor<Component>,
options?: RenderComponentOptions
) => {
const element = document.getElementById('qunit-fixture')!;
element.innerHTML = '';

if (options) {
options.element = element;
await renderComponent(component, options);
} else {
await renderComponent(component, element);
}

return element.innerHTML;
if (options) {
options.element = element;
await renderComponent(component, options);
} else {
await renderComponent(component, element);
}
);

return element.innerHTML;
});
10 changes: 6 additions & 4 deletions packages/@glimmerx/core/tests/modifier-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ import { renderComponent, didRender } from '..';
module('Modifier Tests', () => {
test('Supports the on modifier', async (assert) => {
class MyComponent extends Component {
static template = hbs`<button {{on "click" this.incrementCounter}}>Count: {{this.count}}</button>`;
wondersloth marked this conversation as resolved.
Show resolved Hide resolved
static template = hbs`
<button {{on "click" this.incrementCounter}}>Count: {{this.count}}</button>
`;
@tracked count = 0;

@action
Expand All @@ -19,17 +21,17 @@ module('Modifier Tests', () => {
const element = document.getElementById('qunit-fixture')!;

await renderComponent(MyComponent, element);

assert.strictEqual(
element.innerHTML,
element.innerHTML.trim(),
`<button>Count: 0</button>`,
'the component was rendered'
);

element.querySelector('button')!.click();

await didRender();
assert.strictEqual(
element.innerHTML,
element.innerHTML.trim(),
`<button>Count: 1</button>`,
'the component was rerendered'
);
Expand Down
6 changes: 4 additions & 2 deletions packages/@glimmerx/core/tests/render-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,9 @@ export default function renderTests(

test('a component can use modifiers', async (assert) => {
class MyComponent extends Component {
static template = hbs`<button {{on "click" this.incrementCounter}}>Count: {{this.count}}</button>`;
static template = hbs`
<button {{on "click" this.incrementCounter}}>Count: {{this.count}}</button>
`;

@tracked count = 0;

Expand All @@ -170,7 +172,7 @@ export default function renderTests(
}

const html = await render(MyComponent);
assert.strictEqual(html, `<button>Count: 0</button>`, 'the component was rendered');
assert.strictEqual(html.trim(), `<button>Count: 0</button>`, 'the component was rendered');
});

test('supports functions as simple helpers', async (assert) => {
Expand Down
4 changes: 2 additions & 2 deletions packages/@glimmerx/eslint-plugin/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@
},
"devDependencies": {
"babel-eslint": "^10.1.0",
"eslint": "^6.8.0",
"eslint": "^7.29.0",
"mocha": "^7.1.1"
},
"peerDependencies": {
"@glimmer/syntax": "^0.77.5",
"babel-eslint": "^10.1.0",
"eslint": "^6.8.0"
"eslint": "^7.29.0"
},
"volta": {
"node": "12.10.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/@glimmerx/helper/src/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class BasicHelperManager implements HelperManager<BasicHelperBucket> {
{},
{
get(_target, key) {
return owner && owner.lookup({ type: 'service', name: (key as unknown) as string });
return owner && owner.lookup({ type: 'service', name: key as unknown as string });
},
}
);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
test/fixtures/*/code.*
test/fixtures/*/output.*
63 changes: 63 additions & 0 deletions packages/@glimmerx/prettier-plugin-component-templates/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# prettier-plugin-glimmer-experimental

## Background

[Prettier](https://prettier.io/docs/en/index.html) is an opinionated code formatter. In Prettier `>2.0` there is support for `*.hbs` glimmer files, but does not support the experimental syntaxes that `glimmerx` components are authored in.

## Introduction

This plugin extends the internal printers to add an embedded syntax for glimmer template in an `hbs` TaggedTemplateExpressions.

## Installation and Usage

```bash
yarn add -D @glimmerx/prettier-plugin-component-templates
```

Once added prettier will discover and use the plugin to format any `hbs` tagged template expression.

### Options

In your prettier config options (e.g. `.prettierrc`, `prettier.config.js`, etc.)

`.prettierrc`

```json
{
"hbsSingleQuote": true
}
```

Defaults to `false` forcing double-quotes for all attributes in an hbs embed. When `true` will rewrite all quotes to single-quotes.

## Development

Generate a test case, add it to the tests file,

Add `PRETTIER_DEBUG=true` to the environment when running the plugin in order to get complete stack traces on errors.

### Debugging

#### Debugging

```
PRETTIER_DEBUG=true node --inspect-brk node_modules/.bin/prettier --config=./test/fixtures/basic/config.js ./test/fixtures/basic/code.js
```

### Generate a snapshot

```
node node_modules/.bin/prettier --ignore-path --config=./test/fixtures/simple-formatted/config.js ./test/fixtures/simple-formatted/code.js > ./test/fixtures/simple-formatted/output.js
```

### Testing

#### Run all tests

```
yarn test
```

## TODO

- [ ] Add support for `<template>` tag semantics.
Loading