-
-
Notifications
You must be signed in to change notification settings - Fork 407
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
Component Unification (angle brackets) #60
Conversation
618e7a7
to
0244d63
Compare
0244d63
to
0b32059
Compare
I love the direction with this, but had a few questions while reading:
|
I think they would be an error when used with angle-bracket components.
If you want to describe anything about the component (tags, classes, attributes), a layout is much more expressive than the JS APIs, so "must have a layout" -> "doesn't need the JS APIs" 😉
This is the stickiest question you raised and I don't have great answers yet. To flesh it out:
I don't have a good solution in mind that supports doing all of the above in the same codebase; I kind of feel like you may need to maintain a separate branch for pre-2.0 components 😦 I'm open to anything though, and would love some brainstorming here. |
The main issue isn't with maintaining branches for supporting different Ember versions (that may just be a fact of life in some cases), but for maintaining branches for supporting the two different invocation styles within the same Ember version. |
Another point we likely need to investigate is what happens with double curly components today, that contain a slash. Also what's the order of precedence between native WebComponents and ember component lookup. I suspect ember components should take a higher precedence. But what is the strategy when a collision occurs? What tagName is used? I suspect these collisions will be frustrating to debug if we do not message and provide escape hatches. A finally point is the proposed scoped helper/component yielding + dot syntax. As the dot form is valid, it may be an WebComponents (unless they are more limited then document.createElement) <my-form as |f|>
<f.input>
</my-from> How are theoretical collisions with I suspect the dot versions of tags will not be used, as css rules against them get pretty gnarly |
Does the class get overridden or do they concat when shadowed? It seems like concatting the classes would be very useful for customizing styling of the component, without having to copy all the original styles. |
I'm struggling to see the motivation to unifying 'element' components with 'fragment' components. For me this adds further risks that large changes will come to the component stack when keeping them conceptually separate (perhaps unifying the code base under the hood) will alleviate the risk for change later. Separating a unified concept seems harder than unifying in my opinion. I like the idea of:
This would remove the need for all the bindings as:
Renders:
Partials or fragments could mop up the rest as they currently do. This roughly relates to my attrTypes RFC too: #35 which is designed to help filter attributes passed into the component template (So template authors have to conform to a specified API). This mapping could also be responsible for attributes that don't really make much sense being visible to the DOM like actions (unless a textual alternative could be used instead like a uuid which might help with debugging). |
Also is it worth waiting for the slots syntax to calm down a little as I think this would likely solve the other issues here too. For an easier migration could there be a property I could set on the components JavaScript to disable the replacement? All my components extend from a base level component so it would make a simple transition to disable this (However I would prefer the separation of fragment/partial style components to normal ones). |
which syntax do you have in mind? |
They concat. It would make sense to concat styles too. |
I may have missed something, but with this rule how would one bind a dynamic property to an attribute? i.e. <x-foo aria-label={{someComputedLabel}} /> |
@jgwhite simple: <x-foo aria-label="{{someComputedLabel}}" /> |
@wycats ah, let me check I’ve got this straight:
|
@jgwhite both cases would assign to attrs, but string attrs would also setAttribute. |
Okay cool, that makes sense. Another thing I’m not 100% clear on from the RFC: If I want the in-DOM result of invoking: <x-foo>Bar baz</x-foo> to be: <x-foo><span>Bar baz</span></x-foo> What should |
<x-foo><span>Bar baz</span></x-foo> |
@wycats and if I wanted to recursively invoke |
@jgwhite recursive invocations always require control flow so they wont be at the top of the template. |
@wycats this is making more and more sense as I reason it through. My initial concern was that in templates like this: {{! x-foo.hbs}}
<x-foo>
{{yield}}
{{#if somePredicate}}
<x-foo>Some consequent</x-foo>
{{/if}}
</x-foo> The difference in behaviour between the root |
@jgwhite yeah I worked through it a bunch and came to the conclusion that any plausible recursion case requires some kind of control flow in order to avoid stack overflow. You can think of the outermost tag as the component's signature, and any inner tags as recursive calls. @tomdale thinks we should make this the default conventional approach for templates that don't explicitly require a special tag for some reason. |
Yeah, that’s very clear and very declarative.
Could you elaborate on this point? Do you mean that a conventional component template wouldn’t have an explicit signature element? |
@wycats the slots syntax as is being pushed for Shadow DOM: This appears to be what will land in browsers and I think will solve a lot of the same use cases this is solving. I understand the merit of the duplicate tag within the template of the DOM, however personally that is a whole lot of components that need changing for something that improve only the 'partial' kind. This improvement isn't (as far as I am aware) ever going to be native to HTML. Much like services are essentially just |
@jonathanKingston I'll reply to this more later, but fwiw, I don't think trying to map Ember's semantics very closely to hypothetical web components semantics is a winning strategy. Web Components are still at a very early stage, and I personally think React's experience (and Ember 1.x's, for that matter), bears more heavily on the problem space than a future WC spec. If Ember and React both land on something inexpressible in WC, and it works out smashingly, that would be a good opportunity to attempt to persuade the standards process to make it expressible. |
@wycats My issue isn't in driving the standards development at all with extend the web forward principles. However collapsing both concepts into one before they are complete seems a little premature. Also as far as I was aware there was consensus from major browsers on this syntax. |
emberjs/rfcs#60 This commit implements the proposed semantics for angle-bracket components. The TLDR is that a component’s template represents its “outerHTML” rather than its “innerHTML”, which makes it easier to configure the element itself using templating constructs. Some salient points: 1. If there is a single root element, the attributes from the invocation are copied onto the root element. 2. The invocation’s attributes “win out” over the attributes from the root element in the component’s layout. 3. Classes are merged. If the invocation says `class=“a”` and the root element says `class=“b”`, the result is `class=“a b”`. The rationale is that when you say `class=“a”`, you are really saying “add the `a` class to this element”. A few sticky issues: 1. If the root element for the `my-foo` component is `<my-foo>`, we insert an element with tag name of `my-foo`. While this is intuitive, note that in all other cases, `<my-foo>` represents an invocation of the component. In the root position, that makes no sense, since it would inevitably produce a stack overflow. a. This “identity element” case makes it idiomatic to reflect the name of the component onto the DOM. b. Unfortunately, when we compile a template, we do not yet know what component that template is used for, and, indeed, whether it is even a template for a component at all. c. To capture the semantic differences, we do a bit of analysis at compile time (to determine *candidates* for top-level elements), and use runtime information (component invocation style and the name of the component looked up in the container) to disambiguate between a component’s element and an invocation of another component. 2. If the root element for the `my-foo` component is a regular HTML element, we use the `attachAttributes` functionality of HTMLBars to attach the attributes that the component was invoked with onto the root element. 3. In general, it is important that changes to attributes do not result in a change to the identity of the element that they are contained in. To achieve this, we reused much of the infrastructure built in `buildComponentTemplate`. The consequence of (1) and (2) above is that the semantics are always “a component’s layout represents its outerHTML”, even though the implementation is quite different depending on whether the root element is the same-named component or not.
@stefanpenner: most of your list of objections I actually see as features.
Event listeners on the Component class are fundamentally dubious now that we have the component's own element in the template. You can avoid having two APIs for the same thing and just use
I think moving away from those APIs is positive. When somebody wants a particular tag, it is better to let them just write the tag. So instead of: <some-component tagName="tr" class="tall" /> You can lift the top element out into the caller's context, and implement the component as a fragment: <tr class="tall">
<some-component />
</tr> The reality of the DOM as it works today is that it is not arbitrarily flexible. There are plenty of contexts with strict rules, and plenty of ways to break existing layouts just because you wanted to add a new layer of abstraction. Components are a unit of abstraction that does need to be arbitrarily flexible. They are the function calls in your template. Splitting a big function into smaller functions should not force you to change the output of the function, but that is exactly what happens if components and DOM are forced to be one-to-one. I am also unsympathetic to the web components argument. Ember Components are a superset of web components. We already do much that they don't. But we are still completely interoperable, and you can choose to implement a component in Polymer, and it will work fine with Ember. |
@ef4 this post of yours really turned me around on this RFC |
i like the way this discussion is going! My concerns keep getting widled down, but the remaining are becoming well defined. Lets see where this leads.
I totally agree, also as event-listeners are already contextual, a video tags
I have un-intentionally lumped together tagName and class (legitimate DOM attrs). I wish for us to separate these two, as I also believe tagName usage to be an anti-pattern and your lifting example to be best practices. So lets continue to refine these thoughts, but lets focus on the example I provided. Given: <div class="{{mode}}">
<maybe-fragment-maybe-not hidden=isPresenting />
<video></video>
</div> The lifting strategy, results in defensive structural wrapping. <div class="{{mode}}">
<span hidden=isPresenting >
<maybe-fragment-maybe-not />
</span>
<video></video>
</div> note: It feels wrong, that applying accessibility hints results in potentially breaking defensive structural wrapping Now, after investigation of the components implementation (or via trial & error) a developer may infer that the defensive wrapping is unneeded. Unfortunately Overtime this inference may not continue to stand true as the single-root may transition to multi-root. The transition from single-root to multi root (or back) should be considered poor practice, but the potential does have a non negligible cost. Finally, and what I believe to be the root of my concerns. We have two concepts, range and element, although related they both expose very different and incompatible capabilities. All while occupying the same syntax. <div class="{{mode}}">
{{fragment-thing}}
<video></video>
</div> <div class="{{mode}}">
<not-a-fragment />
<video></video>
</div> aside: I wonder if today |
I like keeping curlies for fragments and angles for components so that the invocation is unambiguous, but it has practical upgrade challenges. We would need a way for the curlies to opt in to one-way binding and own-element-in-template. |
I absolutely agree, @tomdale also voiced this concern. One idea, was to introduce (until we can fully re-claim |
Is there any way to opt in to one way bindings and no attr proxying in curly I'm finding myself writing I know angle brackets will opt in to this but they aren't available yet.. Aside from getting the functionality sooner, would there be any other benefit in having these options available in curlys? If this isn't currently possible and seems like something reasonable I may make an attempt at a PR..? Turning off attr proxying doesn't seem too difficult but will need to look into |
emberjs/rfcs#60 This commit implements the proposed semantics for angle-bracket components. The TLDR is that a component’s template represents its “outerHTML” rather than its “innerHTML”, which makes it easier to configure the element itself using templating constructs. Some salient points: 1. If there is a single root element, the attributes from the invocation are copied onto the root element. 2. The invocation’s attributes “win out” over the attributes from the root element in the component’s layout. 3. Classes are merged. If the invocation says `class=“a”` and the root element says `class=“b”`, the result is `class=“a b”`. The rationale is that when you say `class=“a”`, you are really saying “add the `a` class to this element”. A few sticky issues: 1. If the root element for the `my-foo` component is `<my-foo>`, we insert an element with tag name of `my-foo`. While this is intuitive, note that in all other cases, `<my-foo>` represents an invocation of the component. In the root position, that makes no sense, since it would inevitably produce a stack overflow. a. This “identity element” case makes it idiomatic to reflect the name of the component onto the DOM. b. Unfortunately, when we compile a template, we do not yet know what component that template is used for, and, indeed, whether it is even a template for a component at all. c. To capture the semantic differences, we do a bit of analysis at compile time (to determine *candidates* for top-level elements), and use runtime information (component invocation style and the name of the component looked up in the container) to disambiguate between a component’s element and an invocation of another component. 2. If the root element for the `my-foo` component is a regular HTML element, we use the `attachAttributes` functionality of HTMLBars to attach the attributes that the component was invoked with onto the root element. 3. In general, it is important that changes to attributes do not result in a change to the identity of the element that they are contained in. To achieve this, we reused much of the infrastructure built in `buildComponentTemplate`. The consequence of (1) and (2) above is that the semantics are always “a component’s layout represents its outerHTML”, even though the implementation is quite different depending on whether the root element is the same-named component or not.
emberjs/rfcs#60 This commit implements the proposed semantics for angle-bracket components. The TLDR is that a component’s template represents its “outerHTML” rather than its “innerHTML”, which makes it easier to configure the element itself using templating constructs. Some salient points: 1. If there is a single root element, the attributes from the invocation are copied onto the root element. 2. The invocation’s attributes “win out” over the attributes from the root element in the component’s layout. 3. Classes are merged. If the invocation says `class=“a”` and the root element says `class=“b”`, the result is `class=“a b”`. The rationale is that when you say `class=“a”`, you are really saying “add the `a` class to this element”. A few sticky issues: 1. If the root element for the `my-foo` component is `<my-foo>`, we insert an element with tag name of `my-foo`. While this is intuitive, note that in all other cases, `<my-foo>` represents an invocation of the component. In the root position, that makes no sense, since it would inevitably produce a stack overflow. a. This “identity element” case makes it idiomatic to reflect the name of the component onto the DOM. b. Unfortunately, when we compile a template, we do not yet know what component that template is used for, and, indeed, whether it is even a template for a component at all. c. To capture the semantic differences, we do a bit of analysis at compile time (to determine *candidates* for top-level elements), and use runtime information (component invocation style and the name of the component looked up in the container) to disambiguate between a component’s element and an invocation of another component. 2. If the root element for the `my-foo` component is a regular HTML element, we use the `attachAttributes` functionality of HTMLBars to attach the attributes that the component was invoked with onto the root element. 3. In general, it is important that changes to attributes do not result in a change to the identity of the element that they are contained in. To achieve this, we reused much of the infrastructure built in `buildComponentTemplate`. The consequence of (1) and (2) above is that the semantics are always “a component’s layout represents its outerHTML”, even though the implementation is quite different depending on whether the root element is the same-named component or not.
emberjs/rfcs#60 This commit implements the proposed semantics for angle-bracket components. The TLDR is that a component’s template represents its “outerHTML” rather than its “innerHTML”, which makes it easier to configure the element itself using templating constructs. Some salient points: 1. If there is a single root element, the attributes from the invocation are copied onto the root element. 2. The invocation’s attributes “win out” over the attributes from the root element in the component’s layout. 3. Classes are merged. If the invocation says `class=“a”` and the root element says `class=“b”`, the result is `class=“a b”`. The rationale is that when you say `class=“a”`, you are really saying “add the `a` class to this element”. A few sticky issues: 1. If the root element for the `my-foo` component is `<my-foo>`, we insert an element with tag name of `my-foo`. While this is intuitive, note that in all other cases, `<my-foo>` represents an invocation of the component. In the root position, that makes no sense, since it would inevitably produce a stack overflow. a. This “identity element” case makes it idiomatic to reflect the name of the component onto the DOM. b. Unfortunately, when we compile a template, we do not yet know what component that template is used for, and, indeed, whether it is even a template for a component at all. c. To capture the semantic differences, we do a bit of analysis at compile time (to determine *candidates* for top-level elements), and use runtime information (component invocation style and the name of the component looked up in the container) to disambiguate between a component’s element and an invocation of another component. 2. If the root element for the `my-foo` component is a regular HTML element, we use the `attachAttributes` functionality of HTMLBars to attach the attributes that the component was invoked with onto the root element. 3. In general, it is important that changes to attributes do not result in a change to the identity of the element that they are contained in. To achieve this, we reused much of the infrastructure built in `buildComponentTemplate`. The consequence of (1) and (2) above is that the semantics are always “a component’s layout represents its outerHTML”, even though the implementation is quite different depending on whether the root element is the same-named component or not.
Tried adding a comment to emberjs/ember.js@9e5f072 but no response so thought I would repost here, apologies if this is not the right forum. Trying to use this new implementation but running into an issue (using emberjs/ember.js@04dd84c). I have a component template: <my-component>
<div class="container"></div>
</my-component> and use it in a route/top level template: <my-component model={{model}} /> From the description mentioned in the above commit I would expect that I would end up with a tag in the html but instead I get the error mentioned in emberjs/ember.js@9e5f072. If I remove the And, BTW, this is probably an invalid usage pattern but changing the |
I believe there should be a build time warning or failure if you are using mutative APIs on attrs in angle bracket components. In React props can be mutated as well but their is tooling around this suggests that you should not mutate props. The example below shows mutating both arrays and objects. Arrays are clearly worst that mutating objects as object properties are not propagated. The program becomes hard to reason about if the attrs can be mutated as they are passed along, we should do our utmost to shorten the conceptual gap between the static program and the dynamic process 😉. |
@chadhietala Setting up property descriptors on |
in theory we can make attrs the instance of a class with those descriptors on the proto. So that would only be expensive the first time the class is derived. I suspect in practice this will be few, it would not longer be per snapshot or per component instance. |
@stefanpenner The problem is that Your proposal would require us to cache the class with descriptors on a per-invocation basis, which is not impossible but we don't have any infrastructure for it currently, as best I can tell. Maybe our friends on the Chrome team will hurry up and implement |
yup, but this is both statically known at compile time and there are also less invocation sites then component instances. I suspect we can incur the penalty once per unique signature call-site even at runtime. (although build time deferral is also likely option). (not saying this is the solution we must take, just saying i believe it can be done without a noticeable performance problem) |
Like I mentioned, I believe tooling can be the happy path. In general I don't really believe in the notion of "in dev it's slower". I would rather have consistent performance characteristics regardless of environment. |
Very minor cocern here but with ye olde components, passing in an attr was an effective way to write tests for components that use services.
Any thoughts on whether angle bracket components mean the end of this pattern? This may be another reason that we need a well-paved path for stubbing services. |
@mitchlloyd - I think emberjs/ember-test-helpers#96 is a much better pattern for this (and works with [email protected]+). |
While working to upgrade our app to 2.0 and plan for some future updates, one question came up relating to As a use case, we often provide default values labels for many components, but allow them to be overridden. I'm not even sure if this is the right place to ask this since |
@rwjblue Thanks for the heads up! |
@workmanw You can just do this.foo = 'foo' in this.attrs ? this.attrs.foo : yourDefault; in whatever hook makes sense to you. |
@mmun Okay yea, I was also thinking I could build a computed macro for this use case too. Something like: function attrsDefault(key, defaultValue) {
return Ember.computed('attrs.'+key, function() {
return (key in this.attrs) ? this.attrs[key] : defaultValue;
});
}
Ember.Component.extend({
foo: attrsDefault('foo', 'bar')
}); But I wasn't sure if this would be unadvised or potentially result in a naming conflict for a period of time. |
@workmanw That's definitely something I'm hoping someone experiments with in an addon so we can gather consensus on how useful it is. |
How would angle bracket components work with inverses? For example, I'd write a |
The conversation in this RFC has been great! I think at this point this RFC doesn't reflect current lines of thinking, and either a rewrite or a new RFC will be needed when we are ready to start progessing forward here. I propose that we close this PR... |
Thanks for everyone's hard work here. We have made a ton of progress, and are in the middle of integrating the updated glimmer engine that is required to start working on angle bracket components again. I'm going to close this for now while we continue pushing forward on the known path. We should reopen (or entertain a new RFC) when ready to work on angle bracket components again. |
Rendered