Skip to content

Spreadable Arguments#1

Open
Alonski wants to merge 1001 commits intomasterfrom
argument-spread-splarguments
Open

Spreadable Arguments#1
Alonski wants to merge 1001 commits intomasterfrom
argument-spread-splarguments

Conversation

@Alonski
Copy link
Owner

@Alonski Alonski commented Jun 5, 2019

Copy link

@pzuraq pzuraq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking great so far! Added some comments about where I think this can be fleshed out a bit. I think the next section to start on would be the "How we teach this" section, since that one also doesn't need detailed design work or to understand the internals or anything.


## Motivation

The reason for doing this is to allow less verbose component templates where the verbosity only makes the template harder to read.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I tend to be a bit more on the formal side here, but I think of the summary and the motivation as independent sections. Right now it reads like the summary is the first part of this section - if you remove it, this sentence doesn't make sense any more.

For instance, you could replace this with:

"Spreadable arguments will allow for less verbose component templates, especially where the verbosity..."

And then the sentence (and the section) stand on their own a bit more. Another option would be to start with the current state of the world:

"Currently, users must do x, y, and z in their applications. This is bad for reasons. Spreadable arguments would allow users to solve these problems in a better way..."

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does it feel now?

A possible example in the wild could be
[ember-power-select](https://github.com/cibernox/ember-power-select/blob/master/addon/templates/components/power-select.hbs).
This component has to explicitely pass on many arguments to its child components. With "splarguments" half or more of this template could
be cut down.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I think a very important point to make here is that this also has a knock-on effect which reduces maintenance of components. For instance, currently if I want to wrap a component, I have to explicitly forward every argument:

<Foo @arg1={{@arg1}} @arg2={{@arg2}} />

This is already super verbose and pretty bad, but now let's say that I add @arg3 to Foo - that means that I have to go through and update every single component template that is wrapping Foo.

I think a great example of this would be a Button component, with a bunch of wrapping button "types":

<!-- success-button.hbs -->
<Button @type="success" />
<!-- warning-button.hbs -->
<Button @type="warning" />
<!-- error-button.hbs -->
<Button @type="error" />

This would be a great pattern! No need to subclass the Button component, you can just have a bunch of TO-components that wrap it and provide the @type - until you need to start adding other arguments, like @onClick and so on. Now you have 3 files to go update, which is a giant pain.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to include these examples. What do you think?

@Alonski Alonski changed the title feat: Splarguments first draft Spreadable Arguments Jun 6, 2019
Copy link

@pzuraq pzuraq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, this is fantastic so far! 😄 Love the changes, it reads very well. Let me know if you want help with the detailed design/learning sections!

@Alonski
Copy link
Owner Author

Alonski commented Jun 13, 2019

@pzuraq I would love help. Its finals season for me so my time is more limited than usual. I was planning on working on this a bit on the weekend bit I definitely need help in the detailed design section as well as any other section you can help with :)

@tleperou
Copy link

tleperou commented Jul 3, 2019

I'd link to support you. Should I be a contributor to update the RFC?

@Alonski
Copy link
Owner Author

Alonski commented Jul 4, 2019

@tleperou I am open to PRs to this branch :)

@cibernox
Copy link

cibernox commented Aug 6, 2019

This is more a question than a suggestion. Given that argument are usually passed with curly braces (unless they are strings), would it make more sense to splat inside curly braces?

Example:

<MyComponent {{...arguments}} ...attributes />

Another reason is that, visually, ...arguments and ...attributes are very easy to confuse with each other. {{...arguments}} however stands out more.

@Alonski
Copy link
Owner Author

Alonski commented Aug 6, 2019

@cibernox That looks nice. Chris and I are planning to chat about this in the next few days. Sounds good :)

@pzuraq
Copy link

pzuraq commented Aug 6, 2019

@cibernox I agree that spreading for arguments and attributes needs a way to be syntactically distinct. I actually realized this during the discussion on this thread, where they were suggesting adding a way to spread objects other than the attributes and arguments object, and I think it's very important that we design this feature in such a way that we don't box ourselves into a corner should we decide to add arbitrary spreading in the future.

So, with that in mind, I think there are two syntactically distinct options so far, including yours:

<MyComponent {{...arguments}} ...attributes />

<MyComponent ...@arguments ...attributes />

If we consider the future where we are able to spread arbitrary objects, they look like:

// Variant 1, Option 1
<MyComponent {{...this.myObject}} ...{{this.myAttributes}} />
<MyComponent {{...@myArgObject}} ...{{@myArgAttributes}} />

// Variant 1, Option 2
<MyComponent {{...this.myObject}} ...this.myAttributes />
<MyComponent {{...@myArgObject}} ...@myAttrObject />

// Variant 2
<MyComponent ...@{{this.myObject}} ...{{this.myAttributes}} />
<MyComponent ...@{{@myArgObject}} ...{{@myArgAttributes}} />

Both have their drawbacks and issues. I'm personally slightly in favor of variant 2 for a couple reasons:

  1. Variant 1 looks like it is related to modifiers, which is definitely not true
  2. Variant 2 mirrors the way that arguments are assigned (arg you are assigning is not in curlies, value being assigned is in curlies), but it is a bit ugly, especially in the spreading an argument case.

Definitely open to more suggestions here, we'll probably pick one and list the others as an alternative!

@lougreenwood
Copy link

lougreenwood commented Oct 24, 2019

Just stumbled across this thread again, great work - totally in favour of it!

Just to clarify, in my pre-RFC which @pzuraq linked above, I'm not suggesting arbitrary object splatting, but rather I'm suggesting that we explore some syntax to allow "breaking off" some value from ...attributes, so that it can target separate elements, e.g:

<MyCustomRadio
  class="some-override-styling"
  disabled={{true}}
/>

my-custom-radio.hbs

<input
  {{!-- this only should receive `disabled` attribute --}}
/>

<label
  {{!-- This should receive all attributes EXCEPT `disabled` 
/>

I don't want to derail this PR (so if anyone would like to reply to me, please reply to me on this thread), but just wanted to clarify that the above is the end goal I was seeking to achieve in my pre-RFC - not necessarily splatting of arbitrary objects.

And I'm also not in favour of, nor am I suggesting any special syntax/arbitrary splattable attribute objects, (as suggested above) which would require this to be "routed" at the call site, since it breaks Liskov Substitution Principle with standard HTML elements - for example, this is definitely not what I'm suggesting:

<MyCustomRadio
    attributesForInput={{hash disabled=(true)}}
    attributesForLabel={{hash class="some-override-styling"}}
/>

😉

@Alonski
Copy link
Owner Author

Alonski commented Oct 24, 2019

@lougreenwood I think that what you are not suggesting sounds pretty nice 😅
However its related to attributes and not arguments which as you said should be a different RFC.

A possibility for arguments, and maybe attributes, is this:

<MyComponent @this=that @foo=bar/>

<section {{...arguments (except @foo)}}>

This looks a bit weird though but could potentially do what you are asking.

wagenet and others added 29 commits January 6, 2023 12:19
Co-authored-by: Peter Wagenet <peter.wagenet@gmail.com>
Co-authored-by: Peter Wagenet <peter.wagenet@gmail.com>
Deprecate implicit record loading in Ember Route
@Alonski
Copy link
Owner Author

Alonski commented Feb 4, 2023

@ef4 Did you merge upstream main? Might be better to rebase? Though maybe we don't do that in the Ember repos?
Thanks!

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

Successfully merging this pull request may close these issues.