Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Using "rest arg" version of positionalParams breaks hash only version. #12444

Closed
rwjblue opened this issue Oct 6, 2015 · 4 comments · Fixed by #12445
Closed

Using "rest arg" version of positionalParams breaks hash only version. #12444

rwjblue opened this issue Oct 6, 2015 · 4 comments · Fixed by #12445

Comments

@rwjblue
Copy link
Member

rwjblue commented Oct 6, 2015

Cannot specify positionalParams as a string (which would set the params array to a property of at that location) and specify a hash argument instead of positional params. The assertion added in #12350 is triggered even if no non-hash params are present:

Assertion Failed: You cannot specify positional parameters and the hash argument `allTheThings`.
App.FooBarComponent = Ember.Component.extend();

App.FooBarComponent.reopenClass({
  positionalParams: 'allTheThings'
});
{{foo-bar allTheThings=blah}}

Demo: http://rwjblue.jsbin.com/zeqowe/edit?html,js,output

@mixonic
Copy link
Member

mixonic commented Oct 6, 2015

@rwjblue I do not believe this is true. Input from @ef4 would be welcome, but I think we discussed this when working on (component and agreed this behavior is intentional.

@rwjblue
Copy link
Member Author

rwjblue commented Oct 6, 2015

The entire point of positionalParams is that it is optional, you can invoke with the "long hand" version and it should work exactly the same as if you used the positional params.

Using the example from one of the tests added in #12445:

App.FooBarComponent.reopenClass({
  positionalParams: ['first', 'second']
});

Invoking that component with all three of these lines should result in exactly the same attrs:

{{foo-bar "one" "two"}}
{{foo-bar "one" second="two"}}
{{foo-bar first="one" second="two"}}

The above example works properly today (the test added for that in #12445 is simply confirmation as no changes to the "named" version were made). Using the "rest" argument version should be exactly the same concept: "use positional params if you want, or use hash arguments".

@rwjblue
Copy link
Member Author

rwjblue commented Oct 6, 2015

we discussed this when working on (component and agreed this behavior is intentional

You are thinking about the discussion that lead to #12350, which added assertions if you use the same hash argument as a named positional param. That assertion is still 100% the desired behavior, #12445 makes the named and rest versions of this API the same in that regard.

@mixonic
Copy link
Member

mixonic commented Oct 6, 2015

Sounds good! 👍

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

Successfully merging a pull request may close this issue.

2 participants