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

Adding positional parameter support to components #11028

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
6 participants
@ef4
Contributor

ef4 commented May 5, 2015

A longstanding pattern is to use helpers to simulate components with
positional parameters. But under Glimmer, helpers are pure functions, so
this doesn't work anymore.

Thankfully, we can do something even better, which is to stop wrapping
components in helpers just to get positional params, and instead make
components that can natively accept positional params. That's what this
PR does.

To use, you define your component like:

Ember.Component.extend({
  positionalParams: ['name', 'city']
});

Then you can invoke it like:

{{my-component "Ed" "Somerville"}}

Which is (nearly*) equivalent to:

{{my-component name="Ed" city="Somerville"}}

This depends on a corresponding change in htmlbars.

* The one caveat is that as a new feature, this does not get the legacy auto-mut support that normal attributes get.

Adding positional parameter support to components
A longstanding pattern is to use helpers to simulate components with
positional parameters. But under Glimmer, helpers are pure functions, so
this doesn't work anymore.

Thankfully, we can do something even better, which is to stop wrapping
components in helpers just to get positional params, and instead make
components that can natively accept positional params. That's what this
PR does.

To use, you define your component like:

```js
Ember.Component.extend({
  positionalParams: ['name', 'city']
});
```

Then you can invoke it like:

```handlebars
{{my-component "Ed" "Somerville"}}
```

Which is equivalent to:

```handlebars
{{my-component name="Ed" city="Somerville"}}
```
@ef4

This comment has been minimized.

Show comment
Hide comment
@ef4

ef4 May 5, 2015

Contributor

@rwjblue pointed out that there's another legacy behavior that's not supported for these new positional params: they are not available in the component init method.

Contributor

ef4 commented May 5, 2015

@rwjblue pointed out that there's another legacy behavior that's not supported for these new positional params: they are not available in the component init method.

@ef4

This comment has been minimized.

Show comment
Hide comment
@ef4

ef4 May 5, 2015

Contributor

I think Travis is failing because it's using a cached version of htmlbars and not my updated one.

Contributor

ef4 commented May 5, 2015

I think Travis is failing because it's using a cached version of htmlbars and not my updated one.

@rwjblue

This comment has been minimized.

Show comment
Hide comment
@rwjblue

rwjblue May 5, 2015

Member

Yes, we would have to clear the cache for it to pass. I'd rather merge and release upstream, then update package.json with the new value here.

Member

rwjblue commented May 5, 2015

Yes, we would have to clear the cache for it to pass. I'd rather merge and release upstream, then update package.json with the new value here.

@@ -23,7 +23,7 @@
"express": "^4.5.0",
"github": "^0.2.3",
"glob": "~4.3.2",
"htmlbars": "0.13.12",
"htmlbars": "git://github.com/ef4/htmlbars#component-params-prebuilt",

This comment has been minimized.

@rwjblue

rwjblue May 5, 2015

Member

Please update to 0.13.14, just released upstream PR.

@rwjblue

rwjblue May 5, 2015

Member

Please update to 0.13.14, just released upstream PR.

@rwjblue rwjblue closed this in #11033 May 6, 2015

@Samsinite

This comment has been minimized.

Show comment
Hide comment
@Samsinite

Samsinite May 6, 2015

Would like to see something that allows arbitrary number of params instead... what about supporting an interface that allows the use of arbitrary parameters?

Maybe:

Ember.Component.extend({
  mapPositionalParams: function(foo, bar, ...args) {
    return {
      one: foo,
      two: bar,
      rest: args
    };
  }
});

Would like to see something that allows arbitrary number of params instead... what about supporting an interface that allows the use of arbitrary parameters?

Maybe:

Ember.Component.extend({
  mapPositionalParams: function(foo, bar, ...args) {
    return {
      one: foo,
      two: bar,
      rest: args
    };
  }
});
@Samsinite

This comment has been minimized.

Show comment
Hide comment

@rwjblue @ef4 thoughts?

@ef4

This comment has been minimized.

Show comment
Hide comment
@ef4

ef4 May 10, 2015

Contributor

It would be fairly easy to make varags work like:

Ember.Component.extend({
  positionalParams: ['name', 'city', '...rest']
});
Contributor

ef4 commented May 10, 2015

It would be fairly easy to make varags work like:

Ember.Component.extend({
  positionalParams: ['name', 'city', '...rest']
});
@Samsinite

This comment has been minimized.

Show comment
Hide comment
@Samsinite

Samsinite May 11, 2015

The map function just allows the programmer more flexibility define assignment to the component while still being just as easy to understand with slightly less "magic". The important thing to me though is supporting variadic components.

The map function just allows the programmer more flexibility define assignment to the component while still being just as easy to understand with slightly less "magic". The important thing to me though is supporting variadic components.

@rwjblue

This comment has been minimized.

Show comment
Hide comment
@rwjblue

rwjblue May 11, 2015

Member

I would be in favor of using a string to indicate that all params go to a specific prop to handle the varargs scenario. Something like:

export default Ember.Component.extend({
  positionalParams: 'params'
});

Having this.params (when needing var-args) does seem nicely congruous with this.attrs....

Member

rwjblue commented May 11, 2015

I would be in favor of using a string to indicate that all params go to a specific prop to handle the varargs scenario. Something like:

export default Ember.Component.extend({
  positionalParams: 'params'
});

Having this.params (when needing var-args) does seem nicely congruous with this.attrs....

@Samsinite

This comment has been minimized.

Show comment
Hide comment
@Samsinite

Samsinite May 12, 2015

Sounds good, would you guys like a PR for this? I wouldn't mind adding that support.

Sounds good, would you guys like a PR for this? I wouldn't mind adding that support.

danmcclain added a commit to danmcclain/ember.js that referenced this pull request May 19, 2015

[BUGFIX beta] Allows for an arbitrary # of positional parameters
When the `positionalParams` property on a component is a string, the
positional parameters passed to the component will be an array that is
bound to that attribute, as per requested by @rwjblue here:
emberjs#11028 (comment)

rwjblue added a commit that referenced this pull request Jun 6, 2015

[BUGFIX beta] Allows for an arbitrary # of positional parameters
When the `positionalParams` property on a component is a string, the
positional parameters passed to the component will be an array that is
bound to that attribute, as per requested by @rwjblue here:
#11028 (comment)

(cherry picked from commit 2d128db)
@azizpunjani

This comment has been minimized.

Show comment
Hide comment
@azizpunjani

azizpunjani Aug 12, 2015

@ef4 Could you please give an example of the long standing pattern you mention in the statement "A longstanding pattern is to use helpers to simulate components with
positional parameters. "?

I've looked around to find examples of this pattern you mention but have not found any relevant code.

@ef4 Could you please give an example of the long standing pattern you mention in the statement "A longstanding pattern is to use helpers to simulate components with
positional parameters. "?

I've looked around to find examples of this pattern you mention but have not found any relevant code.

@stefanpenner

This comment has been minimized.

Show comment
Hide comment
@stefanpenner

stefanpenner Aug 12, 2015

Member

I've looked around to find examples of this pattern you mention but have not found any relevant code.

{{each positionalArg}}

{{link-to positionalArg1 positionArg2 etc..}}
Member

stefanpenner commented Aug 12, 2015

I've looked around to find examples of this pattern you mention but have not found any relevant code.

{{each positionalArg}}

{{link-to positionalArg1 positionArg2 etc..}}
@rwjblue

This comment has been minimized.

Show comment
Hide comment
@rwjblue

rwjblue Aug 12, 2015

Member

{{input}}

Member

rwjblue commented Aug 12, 2015

{{input}}

@mmun

This comment has been minimized.

Show comment
Hide comment
@mmun

mmun Aug 12, 2015

Member

{{link-to}}

Member

mmun commented Aug 12, 2015

{{link-to}}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment