-
Notifications
You must be signed in to change notification settings - Fork 235
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add rule
no-builtin-form-components
(#2990)
- Loading branch information
Showing
5 changed files
with
186 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
# no-builtin-form-components | ||
|
||
Ember's built-in form components use two-way data binding, where the property passed as `@value` or `@checked` is mutated by user interaction. This goes against the Data Down Actions Up principle, goes against Glimmer Components’ intention to have immutable arguments, and is [discouraged by the Ember Core team](https://www.pzuraq.com/on-mut-and-2-way-binding/). | ||
|
||
## Examples | ||
|
||
This rule **forbids** the following: | ||
|
||
```hbs | ||
<Input /> | ||
``` | ||
|
||
```hbs | ||
<Textarea></Textarea> | ||
``` | ||
|
||
## Migration | ||
|
||
Many forms may be simplified by switching to a light one-way data approach. | ||
|
||
For example – vanilla JavaScript has everything we need to handle form data, de-sync it from our source data and collect all user input in a single object. | ||
|
||
```js | ||
import Component from '@glimmer/component'; | ||
import { tracked } from '@glimmer/tracking'; | ||
import { action } from '@ember/object'; | ||
|
||
export default class MyComponent extends Component { | ||
@tracked userInput = {}; | ||
|
||
@action | ||
handleInput(event) { | ||
const formData = new FormData(event.currentTarget); | ||
this.userInput = Object.fromEntries(formData.entries()); | ||
} | ||
} | ||
``` | ||
|
||
```hbs | ||
<form {{on "input" this.handleInput}}> | ||
<label> Name | ||
<input name="name"> | ||
</label> | ||
</form> | ||
``` | ||
|
||
Another option would is to "control" the field's value by replacing the built-in form component with a native HTML element and binding an event listener to handle user input. | ||
|
||
In the following example the initial value of a field is controlled by a local tracked property, which is updated by an event listener. | ||
|
||
```js | ||
import Component from '@glimmer/component'; | ||
import { tracked } from '@glimmer/tracking'; | ||
import { action } from '@ember/object'; | ||
|
||
export default class MyComponent extends Component { | ||
@tracked name; | ||
|
||
@action | ||
updateName(event) { | ||
this.name = event.target.value; | ||
} | ||
} | ||
``` | ||
|
||
```hbs | ||
<input | ||
type="text" | ||
value={{this.name}} | ||
{{on "input" this.updateName}} | ||
/> | ||
``` | ||
|
||
You may consider composing the [set helper](https://github.com/pzuraq/ember-set-helper) with the [pick helper](https://github.com/DockYard/ember-composable-helpers#pick) to avoid creating an action within a component class. | ||
|
||
```hbs | ||
<input | ||
type="text" | ||
value={{this.name}} | ||
{{on "input" (pick "target.value" (set this "name"))}} | ||
/> | ||
``` | ||
|
||
## Related Rules | ||
|
||
* [no-mut-helper](no-mut-helper.md) | ||
|
||
## References | ||
|
||
* [Native HTML `input`](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input) | ||
* [Native HTML `textarea`](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/textarea) | ||
* [Native HTML `FormData`](https://developer.mozilla.org/en-US/docs/Web/API/FormData) | ||
* [The `on` modifier](https://guides.emberjs.com/release/components/component-state-and-actions/#toc_html-modifiers-and-actions) | ||
* [ember-headless-form](https://ember-headless-form.pages.dev/) | ||
* [Built-in components guides](https://guides.emberjs.com/release/components/built-in-components/) | ||
* [Built-in `Input` component API](https://api.emberjs.com/ember/release/classes/Ember.Templates.components/methods/Input?anchor=Input) | ||
* [Built-in `Textarea` component API](https://api.emberjs.com/ember/release/classes/Ember.Templates.components/methods/Textarea?anchor=Textarea) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
import Rule from './_base.js'; | ||
|
||
const WHY = 'Built-in form components use two-way binding to mutate values.'; | ||
const ACTION = 'Instead, refactor to use a native HTML element.'; | ||
export const MESSAGES = { | ||
Input: `Do not use the \`Input\` component. ${WHY} ${ACTION}`, | ||
Textarea: `Do not use the \`Textarea\` component. ${WHY} ${ACTION}`, | ||
}; | ||
|
||
const COMPONENTS = new Set(['Input', 'Textarea']); | ||
|
||
export default class NoBuiltinFormComponents extends Rule { | ||
visitor() { | ||
return { | ||
ElementNode(node) { | ||
if (COMPONENTS.has(node.tag)) { | ||
this.log({ | ||
message: MESSAGES[node.tag], | ||
node, | ||
}); | ||
} | ||
}, | ||
}; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
import generateRuleTests from '../../helpers/rule-test-harness.js'; | ||
|
||
generateRuleTests({ | ||
name: 'no-builtin-form-components', | ||
|
||
config: true, | ||
|
||
good: [ | ||
'<input type="text" />', | ||
'<input type="checkbox" />', | ||
'<input type="radio" />', | ||
'<textarea></textarea>', | ||
], | ||
|
||
bad: [ | ||
{ | ||
template: '<Input />', | ||
verifyResults(results) { | ||
expect({ results }).toMatchInlineSnapshot(` | ||
{ | ||
"results": [ | ||
{ | ||
"column": 0, | ||
"endColumn": 9, | ||
"endLine": 1, | ||
"filePath": "layout.hbs", | ||
"line": 1, | ||
"message": "Do not use the \`Input\` component. Built-in form components use two-way binding to mutate values. Instead, refactor to use a native HTML element.", | ||
"rule": "no-builtin-form-components", | ||
"severity": 2, | ||
"source": "<Input />", | ||
}, | ||
], | ||
} | ||
`); | ||
}, | ||
}, | ||
{ | ||
template: '<Textarea></Textarea>', | ||
verifyResults(results) { | ||
expect({ results }).toMatchInlineSnapshot(` | ||
{ | ||
"results": [ | ||
{ | ||
"column": 0, | ||
"endColumn": 21, | ||
"endLine": 1, | ||
"filePath": "layout.hbs", | ||
"line": 1, | ||
"message": "Do not use the \`Textarea\` component. Built-in form components use two-way binding to mutate values. Instead, refactor to use a native HTML element.", | ||
"rule": "no-builtin-form-components", | ||
"severity": 2, | ||
"source": "<Textarea></Textarea>", | ||
}, | ||
], | ||
} | ||
`); | ||
}, | ||
}, | ||
], | ||
}); |