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

Pass attributes to glimmer components when using (component) helper #497

Open
gossi opened this issue Jun 3, 2019 · 45 comments
Open

Pass attributes to glimmer components when using (component) helper #497

gossi opened this issue Jun 3, 2019 · 45 comments

Comments

@gossi
Copy link

gossi commented Jun 3, 2019

This looks like a missing piece to achieve parity between "ember and glimmer" components when using the (component) helper.

With angle brackets and the distinction between arguments and attributes it is no longer possible to pass down attributes to those components. Everything in there will be turned into arguments.

Imagine, I markup my component that way:

<MyComponent @reallyImportantArgument={{this.foo}} disabled={{false}}/>

It will not work with a component helper:

{{component 'my-component' reallyImportantArgument=this.foo disabled=false}}

the latter "param" (disabled=false) will be passed as argument instead of attribute. One workaround is to use the let helper and use angle bracket syntax. However, this does not work when you want to yield a "preconfigured" component, e.g.

{{yield (hash
  MyComponent=(component 'my-component' reallyImportantArgument=this.foo disabled=false)
)}}

I don't know if this is already possible somehow or if this is in a need for a RFC to complete the feature set for octane.

@rwjblue
Copy link
Member

rwjblue commented Jun 3, 2019

It will not work with a component helper:

{{component 'my-component' reallyImportantArgument=this.foo disabled=false}}

... One workaround is to use the let helper and use angle bracket syntax.

Right, you could do something like this:

{{#let (component 'my-component') as |WhateverName|}}
  <WhateverName @reallImportantArgument={{this.foo}} disabled=false />
{{/let}}

But I agree that you are more likely to have issues when passing a "closure component" to someone (not when directly invoking with {{component, like in your example).

@rwjblue
Copy link
Member

rwjblue commented Jun 3, 2019

I do think we'd need an RFC to address this, either new syntax or a custom keyword/helper. Something akin to ember-component-attributes's syntax might work:

{{yield (hash
  MyComponent=(component 'my-component' (html-attributes disabled=false) reallyImportantArgument=this.foo)
)}}

@rwjblue
Copy link
Member

rwjblue commented Jun 3, 2019

@chancancode - Thoughts?

@pzuraq
Copy link
Contributor

pzuraq commented Aug 8, 2019

Could a second positional parameter alone be enough here? I mean, can it be a hash/object? E.g.

{{yield (hash
  MyComponent=(component 'my-component' (hash disabled=false) reallyImportantArgument=this.foo)
)}}

Copy link
Member

rwjblue commented Aug 9, 2019

No, I don't think so because it's perfectly valid to have a component with arbitrary positional params. The reason I suggested a special helper was that we could then add the custom behaviors based on the helper being used instead of arbitrarily "taking over" the second (or last) positional params slot in every component.

@tstormk
Copy link

tstormk commented Aug 14, 2019

I have also been thinking about this, and in my opinion the cleanest solution would be to completely remove the component helper and instead allow standard handlebars to be passed as an argument. Something like this:

{{yield (hash
  MyComponent=(<MyComponent disabled={{false}} @reallyImportantArgument={{this.foo}} />)
)}}

Or:

<MyComponent
  @childComponent={{<OtherComponent />}}
/>

The best part of the new Octane features for me has been the simplification of various Ember features, and I think dynamic component invocation unfortunately is still one of the areas that is lacking in that regard. In fact, introducing angle brackets, named arguments, etc., has made the disparity between regular component invocation and dynamic component invocation worse. Unifying the two would be really nice.

@Exelord
Copy link

Exelord commented Sep 9, 2019

I agree with @tstormk. This would be awesome, but a little hard to imagine. 😅

About @rwjblue proposed solution:

{{yield (hash
  MyComponent=(component 'my-component' (html-attributes disabled=false) reallyImportantArgument=this.foo)
)}}

I think in this case we still wont be able to pass ...attributes to the component :/

Copy link
Member

rwjblue commented Sep 10, 2019

I think in this case we still wont be able to pass ...attributes to the component :/

Hmm, why not? We can make html-attributes (or whatever we settle on) accept ...attributes as a positional param just the same as we can do for angle invocations.

@buschtoens
Copy link
Contributor

The (html-attributes) approach would not support passing element modifiers.

Copy link
Member

rwjblue commented Sep 26, 2019

@buschtoens - You are correct, adding (html-attributes) itself would not be enough to allow modifiers, but when used in conjunction with the (already merged, and currently being implemented) emberjs/rfcs#432 it would work nicely:

{{yield (hash
  MyComponent=(component 'my-component' (html-attributes disabled=false (modifier 'on' 'click' this.onMyComponentClick)) reallyImportantArgument=this.foo)
)}}

@NullVoxPopuli
Copy link
Contributor

NullVoxPopuli commented Sep 26, 2019

could a modifier not have similar usage here? I'm mainly concerned about string representation of invokable things (I know the component helper does exactly this, and I have an idea for that... but I don't want to derail this thread).

{{yield (hash
  MyComponent=(component 'my-component' 
    (html-attributes 
      disabled=false 
      (on 'click' this.onMyComponentClick)
    ) 
    reallyImportantArgument=this.foo
  )
)}}

@rwjblue
Copy link
Member

rwjblue commented Sep 26, 2019

I know the component helper does exactly this, and I have an idea for that

The string part there is unrelated to my point, in strict mode templates (#496) passing a string to the (component, (helper, (modifier will all be disabled.

@rwjblue
Copy link
Member

rwjblue commented Oct 31, 2019

Just wanted to chime in here on my strawman design (in #497 (comment)) and mention that @chancancode had a really good suggestion which wouldn't need to make a "magical" argument to (component:

{{yield (hash
  MyComponent=(with-attributes 
    (component 'my-component' reallyImportantArgument=this.foo)
    disabled=false
  )
)}}

@Exelord
Copy link

Exelord commented Oct 31, 2019

Awesome solution! I'm just wondering how could we pass this was ...attributes? Do we still need some "custom parsing" or can we implement another way?

@ro0gr
Copy link

ro0gr commented Nov 8, 2019

While I really like an elegance of the with-attributes solution, in terms of dev experience, I believe, introducing smth like this would lead to unnecessary complexity in a day to day work.

It happens, when I have to move some of <AngleBracket invocation(or few, and that's where the pain begins) into a {{#let statement, so I can decide whether to yield or render it later in the template.

For instance, the following <AngleBracket:

<MyComponent 
  @class="{{cssScope}}-a"
  @value={{this.a}}
  @onSomething={{action "myAction"}}
/>
<!-- Do not use class attribute to keep parity with curly form -->

in a let statement would look like:

(component "my-component"
  class=(concat cssScope "-a")
  value=this.a
  @onSomething=(action "myAction")
)

Here is a list of manual changes I have to do each time when I just need to put a component to some var(or yield):

  • replace <>|</> with (component ...)
  • rename from CamelCase to dash-erized version of component name
  • remove @s
  • rewrite to (concat
  • remove {{ }}s around values passed to arguments, e.g. ={{this.a}} => =this.a
  • change helpers invocations from {{ }} to ( )

It's error prone.

Also, if we add something like with-attributes, it would add another manual step to the process.

I think, an operation like var assignment should not require any extra skills in dealing wih editor. It should be simple and reasonably fast.

Does anyone have an idea why mixing <Angles within {{curlies is a bad idea? Is it about some handlebars syntax limitations or extra work needed for tools like syntax highlighters?

In case it's a design decision to avoid deeply nested <Angles within curlies, does it sound better if we restrict with only self-closing <Angles /> when inside curlies?

@pzuraq
Copy link
Contributor

pzuraq commented Nov 8, 2019

FWIW @ro0gr we have been discussing better solutions to this, and definitely want to have something better than the component helper as is. This is currently just about closing the gap, so that things that are not possible today become possible again, and folks can actually yield an angle bracket component with feature parity.

In terms of what that better solution could look like, nested angle bracket invocations could work, but we've also discussed potentially having a way to define template blocks inline somehow, and there are other ideas.

The main thing is, we want to land on the right solution here, and we don't want to rush the process, so (with-attributes) is a good stopgap while we figure it out 😄

@ro0gr
Copy link

ro0gr commented Nov 8, 2019

@pzuraq thanks for your response!

I still feel like self-closing(less design decisions needs to consider) angle bracket components could be a good stopgap. And maybe even better.. It just appears to be a natural way to assign AngleBracket to a variable for me 😅

However, it sounds like this option has been considered already, and there must be good reasons to avoid it for now.

@pzuraq
Copy link
Contributor

pzuraq commented Nov 8, 2019

If I remember correctly, there was a concern that it looks too much like an invocation (e.g. no way to distinguish it, except that it happens to be nested), and that it would be a decent amount of work in the parser and potentially the VM. I don't think it's entirely off the table though, it's just not something we can do as a quick, short term fix, and something we really need to think through the implications of.

@robclancy
Copy link

robclancy commented Dec 17, 2019

In angle invocation things are differentiated simply with a @. So having all this extra special stuff seems hack to me.
I think if possible it should just be a syntax change or a prefix. For example attr-disabled=true, :disabled=true [disabled=true].

{{component 'my-component' reallyImportantArgument=this.foo [disabled=false readonly=true]}}

But I do like the idea of the helper going away and instead just using angle syntax but a special version to build the component. Some way to do normal syntax but indicate it isn't to be rendered and just a reference to a pre-configured component (or curried I think you call it?).

{{yield (hash
  MyComponent=(<MyComponent disabled={{false}} @reallyImportantArgument={{this.foo}} />)
)}}

Something like this but with clearer syntax.

I'd also be happy with a helper, I just really need this...

@Panman82
Copy link

What if @ is allowed as a param name? That way params with @ are passed in as args and those without are attributes (as we know it from angle-bracket form). Then modifiers can be passed in after the component name. Ex:

            <MyComponent {{my-modifier "foobar"}} @someArg="foobar" class="foobar" />
{{component "my-component" (my-modifier "foobar") @someArg="foobar" class="foobar"}}

Look fairly close to each other, easy to connect the dots anyway. Just not sure what the transition path would look like since params w/o @ currently go in as args... Also, it would be nice to match component naming conventions, but that's another discussion.

@robclancy
Copy link

robclancy commented Dec 17, 2019 via email

@BobrImperator
Copy link

Personally I don't think that <AngleBracket /> and {{classic-syntax}} should be interchangeable for this.
Unifying conventions as @Panman82 proposed is completely breaking and would require changes in every dynamic component invocation, there's no other way than go invocation by invocation to find out whether disabled === @disabled || disabled === this.disabled for classic class components.
This instead MyComponent=(<MyComponent disabled={{false}} @reallyImportantArgument={{this.foo}} />) is great and tells the story, or what @rwjblue proposed with with-attributes helper, although as a potential user of the api, I think it'd get old quickly

@moet78
Copy link

moet78 commented Aug 27, 2020

Do we have a solution for dynamic component invocation in glimmer yet? Trying to migrate some old app over to glimmer but stuck on this.

@lolmaus
Copy link

lolmaus commented Aug 27, 2020

My impression is that dynamic component invocation is now considered a bad practice. Angle brackets do not provide a way to do it, and the core team does not seem to have an intention to implement it.

Instead, it looks like we are now supposed to use a long {{else if}} chain to invoke components explicitly.

Before:

{{component this.componentName}}

After:

{{#if (eq this.componentName "foo")}}
  <Foo/>
{{else if (eq this.componentName "bar")}}
  <Bar/>
{{else if (eq this.componentName "baz")}}
  <Baz/>
{{/if}}

This can get really verbose and does not substitute for fully dynamic {{component}} cases.

On the plus size, navigating and maintaining the code gets much, much easier. And you get to use attributes, splattributes, modifiers, etc.

PS This is just my impression as a user, it may be false.

@amk221
Copy link

amk221 commented Aug 27, 2020

@lolmaus Why would it be bad practice? What about higher order components like:

{{! parent.hbs }}
{{yield (hash child=(component "child" aria-activedescendant=this.activeChild)}}

(p.s. I'm aware of the static-analysis argument)

@chancancode
Copy link
Member

Angle bracket invocations definitely supports dynamic invocations and has always worked. You just have to use the let helper to name the result of the component helper. See https://github.com/emberjs/rfcs/blob/master/text/0311-angle-bracket-invocation.md#dynamic-invocations

The ability to pass attributes to the component is something else which is what’s being discussed here.

@lolmaus
Copy link

lolmaus commented Aug 27, 2020

@amk221 I didn't mean {{component}} is a bad practice. I mean I have an impression it is to be avoided, like something soon to be deprecated.

One of reasons for this is that it feels like a remnant from the classic Ember era. In addition to simply being old ("morally obsolette" as Russians say), {{component}} forces everything inside it to be converted from curlies to parens, making string concatenation impossible.

It's kinda weird to do an RFC extending {{component}} to support attributes and splattributes. If the core team does want Ember to keep supporting dynamic components, why not instead extend angle brackets to support arbitrary tags and components, e. g.:

<{{this.componentName}}
  @foo="foo {{this.bar}} baz"
  disabled={{true}}
  ...attributes
>
  <{{this.tagName}}/>
</{{this.componentName}}>

@chancancode The only {{#let}} example in the article you linked to is still using the {{component}} helper.

It's also marked with DON'T DO THIS in all caps. This warning might only be about redefining <div> and not related to {{component}}, but it surely does contribute to the impression that {{component}} is to be avoided.

@betocantu93
Copy link
Contributor

betocantu93 commented Feb 7, 2021

This pattern of presetting stuff to dynamic components is common and pretty handy, so having splattributes working for glimmer components would be great, also its a pattern widely used in other UI libraries and so expected from new comers,

{{yield (component "my-component" class="primary")}}

Also expected from other framework devs is not only passing a component of their choosing as argument but a whole "block"

<SomeComponent @append={{
    <ul class="flex"> 
      {{#each this.something as |foo|}} 
        <li> <SomeOtherComponent {{on "click" (fn this.handle foo)}} /> </li> 
      {{/each}}
    </ul>
  }}
/>

This could partially be solved in some scenarios with named blocks, but then you can't add html attributes or modifiers to these the block, one way is to expose internal component api, like

<SomeComponent >
<:append as |api|>
  <ul class="flex {{api.classes}}" {{on "click" api.focus}}> 
      {{#each this.something as |foo|}} 
        <li> <SomeOtherComponent {{on "click" (fn this.handle foo)}} /> </li> 
      {{/each}}
  </ul>
<:append>
</SomeComponent>

Now that I think about it, that's probably a use case worth mentioning. It would be great if we had a standard way to yield modifiers and attributes to be later applied

{{yield (hash 
  events=(array (hash name="click" handler=this.that))
  attributes=(array (hash name="class" value=this.classes))
)}}

Or something like

{{yield (hash 
  events=(to-apply-modifier (on "click" this.that) (on "focus" this.thatFocus))
  attributes=(to-apply-attributes (hash name="class" value="hello"))
)}}
<MyEmptyButton as |api|>
  <a href="#" class="primary" {{html-attributes api.attributes}} {{html-events api.events}}>
    Hello
  </a>
</MyEmptyButton>

Or that the named block have also a ...attributes somehow

<MyEmptyButton>
  <:button>
    <a ...:button>
     
    </a>
  <:/button>
</MyEmptyButton>

Still, in the meantime I partially hacked the splattributes with this approach:
https://twitter.com/betocantu93/status/1358462200416002048

@gossi
Copy link
Author

gossi commented Feb 8, 2021

Would it already be possible today to create a (with-attributes) and (with-modifiers) helper in ember userland space using the same approach as (element) helper? Are all tools/mechanics for that available?

@chancancode

@pzuraq
Copy link
Contributor

pzuraq commented Feb 8, 2021

I believe something like with-attributes and with-modifiers would require changes in the VM itself. The VM internally has a representation of a CurriedComponent, which is a component definition + references to the component's arguments. We would need to add slots for the references to the components attributes, and the curried modifiers for the component as well.

@chancancode
Copy link
Member

we had an idea to extend the element helper for this, and we should be able to polyfill the attributes side with that

@runspired
Copy link
Contributor

with-args would also be useful, often you're in a situation where something else has built up a hash of named args, and currently there's no way to achieve dynamically passing named args without adding them as a single arg.

@JimSchofield
Copy link

I'm curious if anyone has thought to create an ember-native component like Component from Vue?
https://vuejs.org/v2/guide/components.html#Dynamic-Components

I recently encountered the issue of not being able to spread attributes to dynamic component children, and I find that the component helper is inconsistent with normal components. It would be nice to have the following:

<Component
  @is={{@componentName}}
   ...attributes
 />

@NullVoxPopuli
Copy link
Contributor

NullVoxPopuli commented Jun 16, 2021

@JimSchofield, I generally like the idea of finding ways of not using the {{component}} helper 🎉 -- I think an abstraction like that would be fantastic to experiment with in addon-space, as implementation / desired capabilities may vary between greatly between usages (but mostly this is unknown unknown territory?).

An addon could prepare the community for the concept, which could help crowd-source ideas for an RFC to add something to Ember (that maybe does/doesn't use ember-concurrency (or maybe JS will have cancelable promises by then 🙃 ))

for example, I could see this covering the 90%:

// provided by the imaginary addon
// maybe there is some way to check if a module is already loaded, 
// so we can skip the task / loading state altogether, which would
// be more inline with your synchronous `@componentName` example
class DynamicComponent extends Component {
  // useTask from ember-resources
  component = useTask(this, this.dynamicLoader);

   // task used to give us loading/error state "for free"
  @task *dynamicLoader() {
     return yield import(this.args.named);
  }

  <template>
    {{#if this.component.isLoading}}
      {{yield to="loading"}}
    {{else if this.component.isError}}
      {{yield this.component.error to="error"}}
    {{else}}
      {{yield this.component.value to="resolved"}}
    {{/if}}
  </template>
}

// Usage in your design system:
const LoadComponent = <template>
  <DynamicComponent @named={{@named}}>
    <:loading>
      skeleton ui or something
    </:loading>
    <:error as |error|>
      gasp! {{error}}
    </:error>
    <:resolved as |Foo|>
      {{yield Foo}}
    </:resolved>
  </DynamicComponent>
</template>

// Usage of the Usage, for app-devs on your team
<template>
  <LoadComponent @named="my/component/in/the/place" as |Foo|>
     <Foo @arg={{}} class="..." />
  </LoadComponent>
</template>

However, getting there would require:

Sans the <template> stuff, I don't yet know why this couldn't work today

@tylerbecks
Copy link

Hey I just experienced this issue on Ember version 3.20, but I don't have the issue in 3.26. Specifically, the issue is that a Glimmer component invoked with the component helper only has the first argument defined. Was this fixed in a later version of Ember and not backported?

@chriskrycho
Copy link
Contributor

@tylerbecks I think that's a different issue than this one!

@boris-petrov
Copy link

@tylerbecks - I'm sure I remember this being an issue and it being fixed at a later point. So yes, I believe this has since been fixed.

@charlesfries
Copy link

Apologizes in advance if this is the wrong thread for this--any tips on refactoring something like the following?

{{yield (hash
  link-to=(component "link-to" class="class-i-need-on-this-link-to")
)}}

The catch is that I need to infer the route/model/query/etc. args when <LinkTo /> is yielded, while also adding that class as an attr.

@NullVoxPopuli
Copy link
Contributor

infer the route/model/query/etc.

maybe:

import Component from '@glimmer/component';
import { cached } from '@glimmer/tracking';
import { inject as service } from '@ember/service';

// TODO: https://github.com/emberjs/rfcs/issues/657
//   maybe eventually natively on RouteInfo?
function getParams(currentRouteInfo: RouteInfo) {
  let params: Record<string, string | undefined>[] = [];

  while (currentRouteInfo.parent) {
    params = [currentRouteInfo.params, ...params];
    currentRouteInfo = currentRouteInfo.parent;
  }

  return params.map(Object.values).flat();
}

class MyComponent extends Component {
  @service router;
  
  @cached
  get info() {
    let { currentRoute } = this.router;
    
    return {
      routeName: currentRoute.name,
      modelData: getParams(currentRoute),
      queryParams: currentRoute.queryParams,
    }
  }
}
{{yield (hash
  link-to=(component "link-to" 
    route=this.info.routeName 
    models=this.info.modelData 
    queryParams=this.info.queryParams
    class="class-i-need-on-this-link-to")
)}}

?

@lougreenwood
Copy link

we had an idea to extend the element helper for this, and we should be able to polyfill the attributes side with that

@chancancode sounds promising, is there any news on this?

MichalBryxi added a commit to MichalBryxi/ember-popperjs that referenced this issue Dec 4, 2021
- Introduce complete example of <MyMenu> component
- Most of the code taken from:
- https://github.com/GavinJoyce/ember-headlessui/tree/master/tests/dummy/app/components
- Introduce <MyMenu::Item> for styling abilities
- To overcome: emberjs/rfcs#497
@csprocket777
Copy link

After reading through the above history, I'm still left wondering if what I'm trying to do with Ember 3.25 is possible.

I'd like to be able to dynamically render a component and invoke it in the template with a dynamic set of named parameters passed to it. I want my developer user to invoke my component by passing in the name of the dynamic component to render and give it an object outlining the params it should be invoked with.

Is this taboo? Is there something similar that would allow me to do this? I can't yield the component because I'm yielding other content already in the component.

@NullVoxPopuli
Copy link
Contributor

I think some of this is solved by #779 (kinda sorta not really)
e.g.:

class Demo {

  get curried() {
    return 
      <template>
          <TheRealComponent data-foo=curried-attribute ...attributes ...@arguments :::blocks />
      </template>
  }

  <template>
    {{yield (component this.curried curriedA=1 curriedB=2) }}
  </template>
}

But this only works IFF, we can also:

  • pass along all arguments
  • pass along all blocks

@angelayanpan
Copy link

is there any updates on this?

@wagenet
Copy link
Member

wagenet commented Jul 23, 2022

@angelayanpan At this point, it looks like someone needs to actually write an RFC.

@esbanarango
Copy link

👀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests