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

[BUGFIX release] Avoid positionalParams changing attrs after create. #11934

Merged
merged 2 commits into from Jul 31, 2015

Conversation

Projects
None yet
6 participants
@rwjblue
Member

rwjblue commented Jul 31, 2015

Fixes #11686.

Adds deprecation for:

import Ember from 'ember';

export default Ember.Component.extend({
  positionalParams: [ 'a', 'b' ]
});

Deprecation text is:

Calling `var Thing = Ember.Component.extend({ positionalParams: [\'a\', \'b\' ]});`
is deprecated in favor of `Thing.reopenClass({ positionalParams: [\'a\', \'b\'] });

New non-deprecated way:

import Ember from 'ember';

var Thing = Ember.Component.extend();
Thing.reopenClass({
  positionalParams: [ 'a', 'b' ]
});

positionalParams was only introduced in Ember 1.13.0 and is not heavily used, I did a GitHub code search for positionalParams and only 35 results were found.

rwjblue added some commits Jul 31, 2015

[BUGFIX release] Setup named positionalParam array.
Previously, when `positionalParams` was a string value (which means to
set all params as an array at that particular prop in `attrs`) we were
iterating over the length of the destination property name instead of
the available `params`.
@rwjblue

This comment has been minimized.

Show comment
Hide comment
Member

rwjblue commented Jul 31, 2015

@ef4

This comment has been minimized.

Show comment
Hide comment
@ef4

ef4 Jul 31, 2015

Contributor

Reviewed, LGTM. Thanks.

Contributor

ef4 commented Jul 31, 2015

Reviewed, LGTM. Thanks.

@stefanpenner

This comment has been minimized.

Show comment
Hide comment
Member

stefanpenner commented Jul 31, 2015

sg

@stefanpenner stefanpenner merged commit 4373ac6 into emberjs:master Jul 31, 2015

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
@knownasilya

This comment has been minimized.

Show comment
Hide comment
@knownasilya

knownasilya Aug 3, 2015

Contributor

What's the point of this? Seems like it's just making it harder to use (it wasn't used because it wasn't documented before).

Contributor

knownasilya commented Aug 3, 2015

What's the point of this? Seems like it's just making it harder to use (it wasn't used because it wasn't documented before).

@mixonic

This comment has been minimized.

Show comment
Hide comment
@mixonic

mixonic Aug 3, 2015

Member

@knownasilya this is a very power-user feature, and I believe the tradeoff here is ergonomics for performance. Seems a fair swap to me.

Member

mixonic commented Aug 3, 2015

@knownasilya this is a very power-user feature, and I believe the tradeoff here is ergonomics for performance. Seems a fair swap to me.

@mixonic mixonic deleted the rwjblue:make-positional-params-static branch Aug 3, 2015

@mmun

This comment has been minimized.

Show comment
Hide comment
@mmun

mmun Aug 3, 2015

Member

The point is that positional params are a static property of the component class. This change enforces that.

Member

mmun commented Aug 3, 2015

The point is that positional params are a static property of the component class. This change enforces that.

@knownasilya

This comment has been minimized.

Show comment
Hide comment
@knownasilya

knownasilya Aug 3, 2015

Contributor

Makes sense. The guides will have to be updated.

Contributor

knownasilya commented Aug 3, 2015

Makes sense. The guides will have to be updated.

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