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

Redundant removeChildren call? #386

Closed
slegrand45 opened this issue Dec 2, 2015 · 2 comments
Closed

Redundant removeChildren call? #386

slegrand45 opened this issue Dec 2, 2015 · 2 comments

Comments

@slegrand45
Copy link
Contributor

At https://github.com/ocsigen/js_of_ocaml/blob/master/lib/tyxml/tyxml_js.ml#L232, there is a call to the removeChildren function. I was wondering if this call is indeed necessary? Because there is already a removeChildren call at https://github.com/ocsigen/js_of_ocaml/blob/master/lib/tyxml/tyxml_js.ml#L225 for the Set action.

If my understanding is right, the fact that there is this call to removeChildren in the update_children function would cancel the benefits of the diffing patch proposed by @vasilisp (see ocsigen/reactiveData#12).

@vasilisp
Copy link
Contributor

vasilisp commented Dec 2, 2015

update_children is called just once, as an initialization step. The node is subsequently managed reactively. IIRC we need the removeChildren call there, e.g., to remove pre-existing server-side-produced children before the client-side reactive updates start.

With the diffing patch, we will be trigerring the Patch case in merge_msg, where removeChildren does not happen.

I have certainly tested Tyxml_js.R with the diffing patch, and I think it works as expected :).

@slegrand45
Copy link
Contributor Author

Ok, i didn't get it was only call on an initialization step... 😊 Sorry for the noise. Thank you very much for your clarification.

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

2 participants