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

Unable to set parent properties while in init and initial didReceiveAttrs hooks #13823

Closed
btecu opened this issue Jul 14, 2016 · 16 comments
Closed
Labels

Comments

@btecu
Copy link
Contributor

btecu commented Jul 14, 2016

Using Ember 2.6, I'm trying to update a value passed into a component before the component is rendered. I've tried using the hooks that make most sense (init, didReceiveAttrs) but none appear to be able to mutate it. didReceiveAttrsworks on re-renders but not on initial render. didInsertElement works, but it's obviously bad practice and it triggers many deprecations. Any ideas? Is this a bug or something expected?

Here is a twiddle - look in the console log that the observers are never triggered.
https://ember-twiddle.com/1ea116e14e0ea5240859fd29152b2f6f?openFiles=components.x-child.js%2C

@pixelhandler
Copy link
Contributor

pixelhandler commented Jul 15, 2016

@btecu have you found any documentation on how to mutate attributes that are passed in? Seems to me that I would avoid changing an attribute that I passed into the component. I typically just compute properties for the component's use. If the component has setting it needs to do and also receives attributes I treat those as separate concerns. It seems that this example receives attributes and also wants to set those same attributes regardless of what the values were that it received. Is there another way you could solve your use case for this?

It seems the this doc has an example for using didReceiveAttrs https://guides.emberjs.com/v2.6.0/components/the-component-lifecycle/#toc_formatting-component-attributes-with-code-didreceiveattrs-code

@pixelhandler
Copy link
Contributor

pixelhandler commented Jul 15, 2016

@btecu didReceiveAttrs does the trick, see this twiddle: https://ember-twiddle.com/3f92337ff932302dc26556a93698029d?openFiles=templates.components.x-child.hbs%2C

Regarding the component setting the parent (controller) value that should be avoided, you can send an action up if you want the controller to change it's own value.

@btecu
Copy link
Contributor Author

btecu commented Jul 15, 2016

@pixelhandler No it doesn't. In your example is only getting changed locally.

I'm familiar with DDAU and I think in general it's the best way. However, there are some cases where direct binding it's more obvious and cleaner. No need to have an action for just setting a value.

The fact that it works on subsequent calls of didReceiveAttrs makes me think that this is a bug.

@krisselden
Copy link
Contributor

init is for initializing and doesn't fire events, setting values on a parent that are rendered already is deprecated even if it were to fire the change event that would trigger the 2-way binding

@krisselden
Copy link
Contributor

@btecu I do believe that didReceiveAttrs should be called on the init event, the idea is code in didReceiveAttrs should work the same on init and update so it should be called just after the component is fully initialized and fire events.

I personally consider this part to be a bug, it is zalgo to even have this hook if one time it is called it is very different than every other time, if you need init, use init, if you want update only code use update, code in this hook should not work any differently the first time it is called.

@krisselden krisselden reopened this Jul 16, 2016
@krisselden krisselden added the Bug label Jul 16, 2016
@krisselden
Copy link
Contributor

@btecu I'm reopening this specific about didReceiveAttrs, init is definitely not supposed to trigger change events and you absolutely have to be aware of how overriding works, order of how things are initialized and calling super etc. Though if you have bindings to your parent, make sure that it isn't affecting anything already rendered.

@rwjblue
Copy link
Member

rwjblue commented Jul 16, 2016

@krisselden - This is exactly the same thing I tried to fix in #13351 that you argued strongly against. That override is simply wrong (as I stated in that PR), and prevents anything from flowing upstream before the component is inDOM.

@krisselden
Copy link
Contributor

@rwjblue reread my comments, I'm more concerned that observers don't fire, the upstream flow I think is dubious, especially since it is done via a backdoor. The only thing I consider a bug is that didReceiverAttrs fires inside the ctor and leaks a partially initialized object and doesn't fire events, this makes it radically different than when called during update.

@krisselden
Copy link
Contributor

krisselden commented Jul 16, 2016

I would be ok with your other PR if it happened post init during the init event but we've talk about removing upstream flow that affects already rendered state so that would be an issue post init or update which your PR was focused on

@tehmaestro
Copy link

tehmaestro commented Oct 2, 2016

Hi, I've updated from Ember 1.12 to 1.13 and I'm facing the same issue. My use-case is the exact same one describe in this pull request: #13351 (comment) , the AudioPlayerexample.

What is the best way (future proof) of getting the same behaviour as in the AudioPlayer example? Thanks.

@pixelhandler
Copy link
Contributor

@btecu @krisselden @rwjblue is this still an issue, perhaps we should close or create a new reproduction of this, what do you think?

@btecu
Copy link
Contributor Author

btecu commented Sep 19, 2018

Looks like is still an issue with Ember 3.2: https://ember-twiddle.com/1ea116e14e0ea5240859fd29152b2f6f?

@rwjblue
Copy link
Member

rwjblue commented Dec 10, 2018

As of #16185 didReceiveAttrs should be allowed to propagate changes upwards (though beware of backflow during render).

I'm on mobile ATM so its somewhat hard to check the twiddle against latest, @btecu mind double checking with 3.6?

@btecu
Copy link
Contributor Author

btecu commented Dec 11, 2018

@rwjblue I just tested this. init still doesn't propagate changes upwards.
didReceiveAttrs does propagate change upwards but the application errors out as you pointed out:

Error: Assertion Failed: You modified "vDidReceiveAttrs" twice on <zz@controller:application::ember176> in a single render. It was rendered in "component:x-child" and modified in "component:x-child". This was unreliable and slow in Ember 1.x and is no longer supported. See #13948 for more details.

@rwjblue
Copy link
Member

rwjblue commented Dec 11, 2018

Right, that is what #17266 (failing test PR for this scenario) is all about.

Lets track that specific issue over in #17266 though...

@rwjblue rwjblue closed this as completed Dec 11, 2018
@rwjblue
Copy link
Member

rwjblue commented Dec 11, 2018

Also, to clarify:

init still doesn't propagate changes upwards

AFAIK it never has, and I don't think we should change this behavior

didReceiveAttrs does propagate change upwards but the application errors

IMHO, the error is correct. This is actually backflow during render and as such should be avoided...

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

No branches or pull requests

5 participants