Skip to content
This repository has been archived by the owner on Feb 4, 2022. It is now read-only.

Avoid using nested templates when setting innerHTML #411

Merged
merged 2 commits into from
Apr 30, 2018

Conversation

web-padawan
Copy link
Contributor

@web-padawan web-padawan commented Apr 30, 2018

Fixes #410 by applying the change from Polymer/polymer#5210


This change is Reviewable

@e111077
Copy link
Contributor

e111077 commented Apr 30, 2018

You on on point! We were just about to tackle this issue only to find that you've fixed it already with our proposed solution!

Copy link
Contributor

@e111077 e111077 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(temporary "request changes" to block merge while I do some additional checks. Otherwise this looks good)

];
}
const setDisplayNoneStatment = jsc.expressionStatement(jsc.callExpression(
const setDisplayNoneStatement = jsc.expressionStatement(jsc.callExpression(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🍻

@@ -540,7 +540,7 @@ export function createDomNodeInsertStatements(
jsc.callExpression(
jsc.memberExpression(
jsc.identifier('document'), jsc.identifier('createElement')),
[jsc.literal('div')]))]);
[jsc.literal('template')]))]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

Copy link
Contributor

@e111077 e111077 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checks pass

@e111077 e111077 merged commit 15f925f into Polymer:master Apr 30, 2018
@justinfagnani
Copy link
Contributor

Thanks @web-padawan !

@web-padawan web-padawan deleted the fix/insert-template-content branch May 1, 2018 04:51
web-padawan referenced this pull request in Polymer/polymer May 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants