Add new rule no-builtin-form-components
#2990
Conversation
| @@ -0,0 +1,65 @@ | |||
| # no-builtin-form-components | |||
|
|
|||
| Ember's built-in form components use two-way data binding, where the property 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/). | |||
There was a problem hiding this comment.
The pzuraq post is fairly old and mostly concerned with the mut helper. Would be nice to link to a better resource here.
| <Textarea></Textarea> | ||
| ``` | ||
|
|
||
| ## Migration |
There was a problem hiding this comment.
Both of these migration paths are for 'controlled inputs'
There is another migration path, too.
Which is faster, and involves less binding.
See:
There was a problem hiding this comment.
True. I've seen some of this before but never used these patterns exactly. Not sure I understand them well enough make a concise recommendation.
How would you suggest the rule doc is structured?
We might:
- Branch the suggestions based on their existing form type somehow
- Just present both (light/controlled) as options
- Have a "Further reading" discussion section with more details
Ideally the simplest explanation of how to fix this issue is early in this doc.
There was a problem hiding this comment.
Option 2 would be my preference.
You can copy from the above links 🎉
There was a problem hiding this comment.
I've made minor changes to incorporate your recommendations.
To be frank the docs on the ember primitives page are not easy to follow and don't clearly explain the light/controlled concepts so I don't really want to copy that over to this rule doc.
I'd welcome specific suggestions from you, but I can't paraphrase what you want because it's a bit conceptual IMO 😅
For many cases they will be the ideal migration but I think keeping an example of a low-level migration without addons is valuable for users to understand the underlying change that is needed.
There was a problem hiding this comment.
can you open an issue on ember-primitives? docs should be easy to understand, and it's a bug that they're not https://github.com/universal-ember/ember-primitives/ thanks!!
There was a problem hiding this comment.
You got it – universal-ember/ember-primitives#165
|
I'd like to propose this be enabled by default since we're working on the next major anyway |
|
Just to confirm, should this rule be added to |
|
@bmish yeah, i think so. |
gilest
force-pushed
the
no-builtin-form-components
branch
from
312ddee
to
4610195
Compare
| @@ -51,6 +53,8 @@ You may consider composing the [set helper](https://github.com/pzuraq/ember-set- | |||
| /> | |||
| ``` | |||
|
|
|||
| Depending on your requirements, consider using form management patterns like the "light" [Form component from ember-primitives](https://ember-primitives.pages.dev/7-forms/1-intro) or the "controlled" [ember-headless-form](https://ember-headless-form.pages.dev/). | |||
There was a problem hiding this comment.
this isn't quite what I meant about adding. 😅
This is docs, for example -- also in the tutorial
I think we should show an "uncontrolled" example before the controlled example. I think it's important that folks see how to not write JavaScript before they reach for javascript.
I linked to ember-primitives as an example of the pattern, rather than intending for it to be directly linked. Apologies for not being more clear!
There was a problem hiding this comment.
Ahh yes. That makes a lot more sense 😅
FWIW I appreciate the resources you're creating and the modern improvements.
But we should have empathy for the frustrated engineer who is on the receiving end of lint errors, and try to give them a very clear explanation of the problem and how to solve it. Ideally without overloading them with other new ideas.
That's why I'm not using cell or the new component authoring format in the examples.
Anyway I had another run at it. Let me know your thoughts
There was a problem hiding this comment.
FWIW I appreciate the resources you're creating and the modern improvements.
<3
other new ideas.
controlling every aspect of a form is actually the new idea -- the uncontrolled way is the very old way to do forms, it builds on the foundation of the platform and allows the developer to do less work.
Additionally, React has been trying to teach uncontrolled vs controlled forms for 5+ years! 💪
That's why I'm not using cell or the new component authoring format in the examples.
that's totally fine! the details are a means to the concept, and not actually important to the concept -- I think that's probably one thing people get too hung up on when learning new things. It's important to be able to adapt a concept in to your existing code, as not all documentation can match what your existing code is doing.
Anyway I had another run at it. Let me know your thoughts
will do, thanks a ton for working on this and being willing to have the back and forth!!
There was a problem hiding this comment.
a means to the concept, and not actually important to the concept
can match what your existing code is doing.
which you've don't exactly! yay!
gilest
force-pushed
the
no-builtin-form-components
branch
from
e5a2123
to
29dec7f
Compare
gilest
force-pushed
the
no-builtin-form-components
branch
from
29dec7f
to
0f68434
Compare
|
|
||
| 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. |
There was a problem hiding this comment.
this looks great! nice work!
|
From my perspective this is ready @bmish |
bmish
changed the title
no-builtin-form-componentsno-builtin-form-components
Was surprised to find that no such rule exists.
Combined with
no-mut-helperthis can help prevent two-way binding being present in a codebase.Rule docs rendered