Skip to content
This repository has been archived by the owner on Dec 19, 2017. It is now read-only.

deprecate then remove *Changed listeners #120

Closed
DartBot opened this issue Jun 5, 2015 · 13 comments
Closed

deprecate then remove *Changed listeners #120

DartBot opened this issue Jun 5, 2015 · 13 comments

Comments

@DartBot
Copy link

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/1081711?v=3" align="left" width="96" height="96"hspace="10"> Issue by jmesserly
Originally opened as dart-lang/sdk#16993


"Changed" is a magic method suffix:
http://www.polymer-project.org/docs/polymer/polymer.html#change-watchers

it's happened quite a few times that people get errors because of this feature... they expect to use *Changed as an event listener and not have it be magic.

I think we should remove this feature from the Dart port, as it is largely replaced by another feature that is more explicit and IMHO more obvious (@­ObserveProperty -- currently in our under development 0.10.0-dev pkg, inspired by this polymer.js feature: http://www.polymer-project.org/docs/polymer/polymer.html#observeblock), and fits the polymer.dart pattern of using metadata for things recognized by the base class.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/1081711?v=3" align="left" width="48" height="48"hspace="10"> Comment by jmesserly


Removed Type-Defect label.
Added Type-Enhancement label.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/2049220?v=3" align="left" width="48" height="48"hspace="10"> Comment by sigmundch


also - if for some reason we want to keep the feature (compat with polymer.js), then we could be more careful about fooChanged and only doing something special if there is a correspoing observable property named foo.

I also lean towards removing the feature.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/5606182?v=3" align="left" width="48" height="48"hspace="10"> Comment by ussuri


As a user of the feature, I can attest that it's always seemed "too magical" to me. It's seemed to be in a stark contrast with the explicitness of, say, @­CustomTag('my-element') class MyElement. So yes, making it more explicit would be more user-friendly and less error-prone.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/1081711?v=3" align="left" width="48" height="48"hspace="10"> Comment by jmesserly


Removed Area-Polymer label.
Added area-pkg, Pkg-Polymer labels.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/1081711?v=3" align="left" width="48" height="48"hspace="10"> Comment by jmesserly


Removed Library-Polymer label.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/2049220?v=3" align="left" width="48" height="48"hspace="10"> Comment by sigmundch


Added this to the 1.6 milestone.
Removed Priority-Unassigned label.
Added Priority-Medium label.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/2049220?v=3" align="left" width="48" height="48"hspace="10"> Comment by sigmundch


Removed this from the 1.6 milestone.
Added Polymer-P-1 label.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/2049220?v=3" align="left" width="48" height="48"hspace="10"> Comment by sigmundch


Removed Polymer-P-1 label.
Added Polymer-Milestone-Next label.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/2049220?v=3" align="left" width="48" height="48"hspace="10"> Comment by sigmundch


Added PolymerMilestone-Next label.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/2049220?v=3" align="left" width="48" height="48"hspace="10"> Comment by sigmundch


Removed Polymer-Milestone-Next label.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/1081711?v=3" align="left" width="48" height="48"hspace="10"> Comment by jmesserly


@jakemac53 is this fixed in new polymer?

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/984921?v=3" align="left" width="48" height="48"hspace="10"> Comment by jakemac53


Yes in the new polymer these methods will no longer exist

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/984921?v=3" align="left" width="48" height="48"hspace="10"> Comment by jakemac53


Closing this because I will forget later

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

No branches or pull requests

1 participant