Skip to content
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

add init flag to observers #1205

Closed
krisselden opened this issue Jul 23, 2012 · 24 comments
Closed

add init flag to observers #1205

krisselden opened this issue Jul 23, 2012 · 24 comments
Assignees

Comments

@krisselden
Copy link
Contributor

Add a flag to indicate an observer function should be called on init.

Ember.Object.extend({
    selectionDidChange: function() {

    }.observes('selection').init() // auto call function on init
});
@workmanw
Copy link

+1 . 90% of the time I implement init: function() {} it looks like this.

Ember.Object.extend({
  init: function() {
    this._super();
    this._somethingDidChange();
    this._somethingElseDidChange();
  }
});

@lukemelia
Copy link
Member

+1 same as @workmanw

@ebryn
Copy link
Member

ebryn commented Jul 23, 2012

👍

@sly7-7
Copy link
Contributor

sly7-7 commented Jul 23, 2012

@pangratz
Copy link
Member

🙌

@lcoq
Copy link
Contributor

lcoq commented Jul 24, 2012

@sly7-7 it does! 👍

@wagenet
Copy link
Member

wagenet commented Jul 25, 2012

👎 I know this was discussed some with @tomhuda, though I'm not sure what the conclusion was. For my part, it's not at all obvious what this function would do. I don't see any major burden in putting things in the init method, which seems like a more correct approach anyway.

@lukemelia
Copy link
Member

I agree the naming is confusing. Perhaps runOnInit() is better. But it feels like a simple feature that eliminates boilerplate that a lot of people encounter (as evidenced by the +1s). That seems very much in keeping the Ember philosophy.

@wagenet
Copy link
Member

wagenet commented Jul 25, 2012

If we do anything with this, it should be after we decide on whether we alter the behavior of create.

@krisselden
Copy link
Contributor Author

I do think it would be simpler if the super() call weren't there, if there was a postInit() hook or the current init() becomes _init() so init() doesn't need super called.

I still think people forget to think about init when creating observers.

@workmanw
Copy link

I'm not sure what the proposed alteration to create is, but with the way things work now I find value in this. I don't think there is much of a burden with putting things in the init, but the same could really be said for observers and bindings too. I just see it as an extra convenience, just saves me a little bit of time and makes my code a little more readable.

Ember.Object.extend({
  init: function() {
    this._super();
    this._somethingDidChange();
  }
  /** VS **/
  _somethingDidChange: function() {

  }.observes('something').onInit()
});

I think onInit() might be a good name, short and descriptive.

@wagenet
Copy link
Member

wagenet commented Jul 25, 2012

@workmanw The proposed change to create is to have it behave more like setProperties. I suspect that if we made that change, initial observers would be fired so this would become a non-issue.

@krisselden
Copy link
Contributor Author

@wagenet that proposal was made also with the suggestion the object not fire observers on init. If setProperties from create() caused observers you would still need to manually init for extend, undefined -> defined would still not be consistently a change.

onInit isn't that much clearer, autoInit maybe the most clear and terse.

create brings you to state 0, and then observers handle it after that.

@wagenet
Copy link
Member

wagenet commented Dec 19, 2012

@kselden, are your thoughts on this still the same?

@stefanpenner
Copy link
Member

if @kselden's thoughts are the same, we should make sure our use of `autoinit is consistent.

Then the recent change: autoInit -> autoinit should be reflected here as observes('..').autoinit()

@lukemelia
Copy link
Member

If this is desired functionality, please assign me and I will implement. Otherwise, let's close this since it has been hanging out for 9 months.

I like this feature and would like to see it added.

@ghost ghost assigned lukemelia Jun 13, 2013
@stefanpenner
Copy link
Member

oh the world of open source, being able to assign tasks to your CTO. 👍 Luke you have been assigned!

@wagenet
Copy link
Member

wagenet commented Jul 19, 2013

@lukemelia Are you going to do this?

@knownasilya
Copy link
Contributor

👍

@guilhermeaiolfi
Copy link

Well, I thought that observers already did that: being fired after creation. I've found this issue after creating a complex component and observing its value property for doing other stuff. Every time ember transition to a view that shown that component, its value changes, but the observer is never called b/c its value is set on creation.

So +1 from me. Except that I don't think we need the flag, it should be the default behavior.

@lukemelia
Copy link
Member

This functionality is provided for in PR #3155, in a more general way. If that PR is merged, you can do:

Ember.Object.extend({
    selectionDidChange: function() {
      // this method will be called when the object is created, as well as when selections changes
    }.observes('selection').on('didInit')
});

We can easily sugar that to .autoinit() or something similar, but let's wait and see if it's necessary.

@wagenet
Copy link
Member

wagenet commented Aug 18, 2013

@guilhermeaiolfi, observers are NOT fired after creation. If you want to ensure that they are, you need to call them from the init hook.

@guilhermeaiolfi
Copy link

@wagenet I know that now. I was saying exactly that I think it SHOULD be fired after creation. I mean, only if the creation of that object sets the observed property (directly or by bindings).

@abrahamgreyson
Copy link

🙌

rwjblue added a commit that referenced this issue Nov 20, 2020
Includes the following bugfixes:

* [#1209](glimmerjs/glimmer-vm#1209) Ensure `<output form="some-value">` works properly
* [#1205](glimmerjs/glimmer-vm#1205) Ensure `@tracked` assertion can be made a deprecation
* [#1204](glimmerjs/glimmer-vm#1204) Ensure `loc` is populated by `build ers.element(...)`

Changelog here:

https://github.com/glimmerjs/glimmer-vm/releases/tag/v0.66.1
rwjblue added a commit that referenced this issue Nov 23, 2020
Includes the following bugfixes:

* [#1209](glimmerjs/glimmer-vm#1209) Ensure
  `<output form="some-value">` works properly
* [#1205](glimmerjs/glimmer-vm#1205) Ensure
  `@tracked` assertion can be made a deprecation

Changelog here:

https://github.com/glimmerjs/glimmer-vm/releases/tag/v0.65.1
rwjblue added a commit that referenced this issue Nov 23, 2020
Includes the following bugfixes:

* [#1209](glimmerjs/glimmer-vm#1209) Ensure `<output form="some-value">` works properly
* [#1205](glimmerjs/glimmer-vm#1205) Ensure `@tracked` assertion can be made a deprecation

Changelog here:

https://github.com/glimmerjs/glimmer-vm/releases/tag/v0.62.5
sly7-7 pushed a commit to sly7-7/ember.js that referenced this issue Mar 10, 2021
Includes the following bugfixes:

* [emberjs#1209](glimmerjs/glimmer-vm#1209) Ensure `<output form="some-value">` works properly
* [emberjs#1205](glimmerjs/glimmer-vm#1205) Ensure `@tracked` assertion can be made a deprecation
* [emberjs#1204](glimmerjs/glimmer-vm#1204) Ensure `loc` is populated by `build ers.element(...)`

Changelog here:

https://github.com/glimmerjs/glimmer-vm/releases/tag/v0.66.1
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

No branches or pull requests