Skip to content

Conversation

@divdavem
Copy link
Member

This pull request fixes an issue raised by @pgrab: it is not possible to update bindings from the $dispose method in a component, because the observer is removed before calling $dispose.

Before integrating this pull request, I still have to:

  • add a test case

@divdavem
Copy link
Member Author

I have opened this pull request because it seemed to fit the use case from @pgrab, but after thinking a bit, I am not convinced at all it is a good thing to update the data model in the $dispose method.
Especially, the $dispose method is often called because of a change in the data model, and, probably, the new state of the data model is no longer ready to receive the value.

Let's consider the following template:

# template main(data)
    {if data.document.type == "text"}
        <#textareaWithOnDispose text="{data.document.content}"/>
    {else if data.document.type == "html"}
        <#htmlComponentWithOnDispose html="{data.document.content}"/>
    {/if}
# /template

With this template, let's say we have data.document.type == "text". Then the user changes the content of the textarea (it is not saved yet in the data model, because the controller only saves value in the data model in $dispose).
Then, later, there is some code which sets data.document = myNewHTMLDocument, knowing that myNewHTMLDocument.type == "html". This operation calls $dispose in textareaWithOnDispose, but the new value of data.document is already present in the data model, and setting the text attribute will change myNewHTMLDocument.content and not the content property of the old document. So I am thinking that changing the data model in $dispose is a bit dangerous and we should probably avoid it.

@b-laporte @pgrab What do you think? So should I simply close this pull request?

@pgrab
Copy link

pgrab commented Apr 25, 2014

I'm fine with your first proposal of still having the observer working when you're inside the dispose method. The drawback you raise into your comment is also true, but it's more about a design issue of the application using the framework than the framework itself. In my case I'm using a third party component that is editable, so basically I have to connect manually the data from the hashspace model to it. And since it's in one tab when I switch and switch back not having the capacity to update the hashspace model inside the dispose method leads me to loose the data. The other way to do it would be to update the hashspace model at each change into the component itself, which is not necessary in my case and is an assumption on the third party component to be able to manage this kind of interaction that is not necessary. So I'm fine with the change (and it works for me ;-) ).

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

Successfully merging this pull request may close these issues.

2 participants