X Tutup
Skip to content

feat(renderer): use a comment instead of an element when stamping out…#5227

Closed
tbosch wants to merge 1 commit intoangular:masterfrom
tbosch:vc_anchor
Closed

feat(renderer): use a comment instead of an element when stamping out…#5227
tbosch wants to merge 1 commit intoangular:masterfrom
tbosch:vc_anchor

Conversation

@tbosch
Copy link
Copy Markdown
Contributor

@tbosch tbosch commented Nov 10, 2015

<template> elements

Closes #4805

@tbosch tbosch added action: review The PR is still awaiting reviews from at least one requested reviewer pr_state: LGTM and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Nov 10, 2015
@tbosch
Copy link
Copy Markdown
Contributor Author

tbosch commented Nov 11, 2015

Reviewed in person with @vsavkin

@tbosch tbosch assigned tbosch and unassigned vsavkin Nov 11, 2015
@tbosch tbosch added this to the beta-00 milestone Nov 11, 2015
@tbosch
Copy link
Copy Markdown
Contributor Author

tbosch commented Nov 13, 2015

/cc @vsavkin: Could you merge this?

@vsavkin vsavkin added the action: merge The PR is ready for merge by the caretaker label Nov 13, 2015
@mary-poppins
Copy link
Copy Markdown

Merging PR #5227 on behalf of @vsavkin to branch presubmit-vsavkin-pr-5227.

@mary-poppins mary-poppins removed the action: merge The PR is ready for merge by the caretaker label Nov 13, 2015
@vsavkin vsavkin added the action: merge The PR is ready for merge by the caretaker label Nov 14, 2015
@mary-poppins
Copy link
Copy Markdown

Merging PR #5227 on behalf of @vsavkin to branch presubmit-vsavkin-pr-5227.

@mary-poppins mary-poppins removed the action: merge The PR is ready for merge by the caretaker label Nov 14, 2015
@tbosch tbosch closed this in bb9cfe6 Nov 14, 2015
@vsavkin vsavkin reopened this Nov 16, 2015
@vsavkin vsavkin assigned jeffbcross and unassigned tbosch Nov 16, 2015
@vsavkin
Copy link
Copy Markdown
Contributor

vsavkin commented Nov 16, 2015

Use comments does not work with AppView.logBindingUpdate.

@jeffbcross
Copy link
Copy Markdown
Contributor

FYI I am planning to work on this after landing #5036, which hopefully will be done today (work is done, just updating docs now and then need to submit PR). The time risk of that issue depends mostly on how difficult it is to merge into google3.

@jeffbcross
Copy link
Copy Markdown
Contributor

Back to work on this issue now...

@jeffbcross
Copy link
Copy Markdown
Contributor

To recap the approach I've just discussed with @tbosch, for my own memory:

I'm going to implement a new Renderer method called setBindingDebugInfo that will be called by AppView.prototype.logBindingUpdate. The setBindingDebugInfo method will check if the element is a comment, try to extract a JSON object containing its bindings. Then the updated bindings will be serialized back to the comment as a JSON object: <!--template bindings={"property":"value"}-->

To support this, when the template comment is initially created, it will contain an empty bindings object: <!--template bindings={}-->

The setBindingDebugInfo method in the web worker renderer will serialize the inputs and tell the main thread to execute setBindingDebugInfo.

We decided on implementing the new method so we can not negatively impact the performance of the setElementAttribute method that is currently being called by logBindingInfoUpdate, since the use case of updating binding info is only for dev mode.

jeffbcross added a commit to jeffbcross/angular that referenced this pull request Nov 20, 2015
This is used for setting property binding values as attributes
on elements when running in dev mode. This implementation will
also serialize binding information to template placeholder
comment nodes.

Closes angular#5227
@ericmartinezr
Copy link
Copy Markdown
Contributor

Sorry for bumping. The changelog for alpha 47 shows that this was closed in this commit bb9cfe6, but I still can see script tags in the DOM
Repro with a47
http://plnkr.co/edit/M37R7sk1tI3mwwrGgLo4?p=preview

Is that an error in the changelog?

@jeffbcross
Copy link
Copy Markdown
Contributor

Oops that is a problem with our change log script not knowing when a commit
has been reverted. The change had been merged and reverted so is currently
still pending.
On Tue, Dec 1, 2015 at 2:04 PM ericmartinezr notifications@github.com
wrote:

Sorry for bumping. The changelog for alpha 47 shows that this was closed
in this commit bb9cfe6 bb9cfe6,
but I still can see script tags in the DOM
Repro with a47
http://plnkr.co/edit/M37R7sk1tI3mwwrGgLo4?p=preview

Is that an error in the changelog?


Reply to this email directly or view it on GitHub
#5227 (comment).

vsavkin pushed a commit to vsavkin/angular that referenced this pull request Dec 10, 2015
This is used for setting property binding values as attributes
on elements when running in dev mode. This implementation will
also serialize binding information to template placeholder
comment nodes.

Closes angular#5227
@tbosch tbosch deleted the vc_anchor branch December 11, 2015 23:31
@angular-automatic-lock-bot
Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

css :first-child selector will not work when using *ng-for

6 participants

X Tutup