-
-
Notifications
You must be signed in to change notification settings - Fork 408
Default Component Manager #758
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,138 @@ | ||
| --- | ||
| Stage: Accepted | ||
| Start Date: 2021-05-17 | ||
| Release Date: Unreleased | ||
| Release Versions: | ||
| ember-source: vX.Y.Z | ||
| ember-data: vX.Y.Z | ||
| Relevant Team(s): Ember.js, Learning | ||
| RFC PR: https://github.com/emberjs/rfcs/pull/758 | ||
| --- | ||
|
|
||
| # Default Component Manager | ||
|
|
||
| ## Summary | ||
|
|
||
| Anything that can be in a template has its lifecycle managed by the manager pattern. | ||
| Today, we have known managers for `@glimmer/component`, `@ember/helper`, etc. | ||
| But what happens when the VM encounters an object for which there is no manager?, | ||
| such as a plain function? This RFC explores and proposes a default behavior for | ||
| those unknown scenarios when it comes to _components_. | ||
|
|
||
| ## Motivation | ||
|
|
||
| The addon, [ember-could-get-used-to-this](https://github.com/pzuraq/ember-could-get-used-to-this) | ||
| demonstrated that it's possible to use plain functions for helpers and modifiers. | ||
| Components on the other hand, have had no such "plain" / "vanilla" support / adoption from addons. | ||
| Maybe it's worth exploring what default behavior components should have. | ||
|
|
||
| This could reduce the number of dependencies folks need to worry about when building components. | ||
| Using native classes eliminates the reliance on a "framework thing" and further | ||
| lessens the appearance of "magic through object-oriented inheritance". | ||
|
Comment on lines
+30
to
+31
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I very strongly disagree here. This is actually much more "magical": in this design, the framework calls a class constructor with specific arguments because it happens to be invoked in a specific way. It's not equivalent to function-based helpers or modifiers in my view:
To be sure, I can see a way to rationalize this in terms of saying that a component template is simply a specific kind of callback to the rendering engine (in a real sense that is what it is, of course), but it’s very strange notionally, and seems to me to move away from the actual value of explicitness that the Glimmer component subclass gives us. I also don't think that there's anything particularly object-oriented about this use of inheritance, nor that it's in the realm of anti patterns that we do see from inheritance-based designs. There is effectively no behavior we're worried about here: just some boilerplate elimination via setting up the Insofar as one potential goal might be getting rid of the |
||
|
|
||
| Additionally, a goal of baseclass-less-classes as components is to have 0 potential API | ||
| conflicts with userspace. | ||
|
Comment on lines
+33
to
+34
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In general, it's rare to want to name something as abstract as import GlimmerComponent from '@glimmer/component';
export default class Component extends GlimmerComponent {
// ...
}Accordingly, I don't think this is actually a valuable goal. Names are actually useful and good things to provide, and they provide us great hooks for teaching, for docs, etc.! |
||
|
|
||
| ## Detailed design | ||
|
|
||
| Each implemented meta-manager needs a default manager specified. The logic for choosing when to use | ||
| a default manager is, at a high-level: | ||
|
|
||
| ``` | ||
| if (noExistingManagerFor(X)) { | ||
| return defaultManager; | ||
| } | ||
| ``` | ||
| Where X is, for the purposes of this RFC, a Component. | ||
|
|
||
| _A Default Manager is not something that can be chosen by the user, but is baked in to the framework | ||
| as a default so that a user doesn't have to build something to use a non-framework-specific variant | ||
| of the three constructs: Helpers, Modifiers, and Components._ | ||
|
|
||
|
|
||
| The default component manager could be similar to the component manager for `@glimmer/component`, | ||
| but be compatible with vanilla classes, such as: | ||
| ```js | ||
| class MyComponent { | ||
| @tracked count = 0; | ||
| } | ||
| ``` | ||
| Since the introduction of `@ember/destroyable`, we no longer _need_ a specific class for helping | ||
| with destruction. | ||
|
|
||
| If a component wanted to have a destruction hook, it could register one itself during construction: | ||
| ```js | ||
| import { registerDestructor } from '@ember/destroyable'; | ||
|
|
||
| export default MyComponent { | ||
| constructor(...args) { | ||
| super(...args); | ||
|
|
||
| registerDestructor(this, () => this.myDestroy) | ||
| } | ||
|
|
||
| myDestroy() { /* ... */ } | ||
| } | ||
| ``` | ||
|
|
||
| and the component manager to support this could look like: | ||
|
|
||
| ```js | ||
| import { setComponentManager } from '@ember/component'; | ||
| import { destroy } from '@ember/destroyable'; | ||
|
|
||
| class DefaultComponentManager { | ||
| constructor(owner) { this.owner = owner; } | ||
|
|
||
| createComponent(ComponentClass, args) { | ||
| return new ComponentClass(this.owner, args); | ||
| } | ||
|
|
||
| getContext(component) { | ||
| return component; | ||
| } | ||
|
|
||
| destroyComponent(component) { | ||
| destroy(component); | ||
| } | ||
| } | ||
|
|
||
| // side-effect -- this file needs to be imported for the component manager to be installed | ||
| setComponentManager((owner) => new DefaultComponentManager(owner), Object.constructor /* TBD */); | ||
| ``` | ||
|
|
||
| ## How we teach this | ||
|
|
||
| The default component manager may not need its own guides-documentation, because the prevelance | ||
| of `@glimmer/component` is so widespread, documenting yet-another-way-to-define-a-component may | ||
| cause more harm than good. | ||
|
|
||
| Existing users of Ember should be made aware of these capabilities, once implemented, via | ||
| the release blog post, with some examples -- but folks watching developments in the ecosystem | ||
| will likely be aware of ember-could-get-used-to-this and ember-modifier, which both implement | ||
| some parts of this RFC, so the migration path for users of those addons should be straight-forward. | ||
| Those addons _may_ want to deprecate their similar functionality after the proposed behavior here | ||
| lands -- though, this RFC suggests implementation such that developers may still use both | ||
| addons without disruption. | ||
|
|
||
|
|
||
| ## Drawbacks | ||
|
|
||
| People may begin to ask if they even need `@glimmer/component`, and that's | ||
| something that the core team may want to address -- some folks may feel like there is churn | ||
| in the component base-class, and don't know "what is correct", etc. The two approaches are | ||
| similar enough where we could introduce a migration via lint rule or codemod. | ||
|
|
||
| ## Alternatives | ||
|
|
||
| The primary alternative to class-based components that folks are aware of is function based-components | ||
| (though this would force the introduction of new paradigms, e.g.: React's hooks) | ||
| - React's hooks + ember's reactivity _may_ be something with exploring further and there have been a couple | ||
| of experiments already: | ||
| - https://github.com/rwjblue/ember-functional-component | ||
| - https://github.com/lifeart/hooked-components | ||
| - https://github.com/NullVoxPopuli/ember-function-component | ||
|
|
||
| ## Unresolved questions | ||
|
|
||
| TBD | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is aimed at reducing the number of imports, then it has an interesting side effect: it makes the “base case” more for people to type. It's extremely rare in my experience to want a backing class which doesn't care at all about
args. Given that, the proposed delta for your average component would be from today’s default——to this:
This is a substantial increase in lines of code, and the result is a substantial decrease in the basic safety: you can accidentally end up just stomping
this.argssomewhere by writingthis.args = 'lol';: something Glimmer component protects you from.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's a good point 🤔