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

Use javascript string escaping in Polymer.html #5062

Merged
merged 2 commits into from
Jan 26, 2018
Merged

Conversation

dfreedm
Copy link
Member

@dfreedm dfreedm commented Jan 25, 2018

There's a lot of pitfalls

Fixes #5060

Copy link
Contributor

@TimvdLippe TimvdLippe left a comment

Choose a reason for hiding this comment

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

LGTM. Lets see what @justinfagnani thinks as well.

Copy link
Member

@kevinpschaaf kevinpschaaf left a comment

Choose a reason for hiding this comment

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

LGTM

@kevinpschaaf
Copy link
Member

Went over the tradeoffs with a few people in the bullpen. There are a few considerations.

Pros for using raw:

  • Can mostly use HTML escaping rules, not JS escaping rules (e.g., you can safely write e.g. <div>Are you happy (yes\no)?</div> in the HTML without escaping the \ since the \ means nothing special in HTML)

Cons for using raw:

  • You can't get around needing to escape a couple of JS-specific characters in the HTML, even when using raw: e.g. ` and $ when preceeding a { still needs to be escaped when used in HTML to prevent closing the string or starting a part. For example, a common footgun would be <div>The price is ${{price}}</div>; the $ needs to be escaped to \${{price}} per JS rules.
  • The \ required to escape the backtick and dollar signs per the above would need to be post-processed out of the raw text, since they still show up in the raw output, making the processing in the html tag function more complex
  • Since most users are familiar with backticked-strings conforming to JS escaping rules, having mostly non-JS escaping rules, but needing to remember a couple of them just sounds confusing

@kevinpschaaf kevinpschaaf merged commit b3dad92 into master Jan 26, 2018
@kevinpschaaf kevinpschaaf deleted the remove-strings-raw branch January 26, 2018 18:43
@usergenic
Copy link

👏

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

Successfully merging this pull request may close these issues.

5 participants