Skip to content

fix: nested embeds (JS in HTML in JS)#6038

Merged
duailibe merged 8 commits intoprettier:masterfrom
thorn0:fix-nested-embeds
May 27, 2019
Merged

fix: nested embeds (JS in HTML in JS)#6038
duailibe merged 8 commits intoprettier:masterfrom
thorn0:fix-nested-embeds

Conversation

@thorn0
Copy link
Copy Markdown
Member

@thorn0 thorn0 commented Apr 7, 2019

Currently, if JS code embedded in HTML (via <script>) embedded in JS (via a template literal) contains template literals, the inner JS isn't formatted.

const html = /* HTML */ `<script>var a=\`\`</script>`;

(playground)

This fix has been extracted from #5993.

Try the playground for this PR

@duailibe
Copy link
Copy Markdown
Collaborator

Sorry this was overlooked @thorn0

Can you add an entry to the CHANGELOG?

@duailibe
Copy link
Copy Markdown
Collaborator

duailibe commented May 23, 2019

FTR this doesn't work (not saying you should fix it)

Prettier pr-6038
Playground link

--parser babylon

Input:

const html = /* HTML */ `<script>const html = /* HTML */ \`<script></script>\`;</script>`;

Output:

SyntaxError: Unexpected closing tag "script". It may happen when the tag has already been closed by another tag. For more info see https://www.w3.org/TR/html5/syntax.html#closing-elements-that-have-implied-end-tags (1:53)
> 1 | <script>const html = /* HTML */ `<script></script>`;</script>
    |                                                     ^

I guess I'm wondering how far we need to go to fix this.

Is this blocking you ? Do you have "JS in HTML in JS" ?

@thorn0
Copy link
Copy Markdown
Member Author

thorn0 commented May 23, 2019

I have a bit of "JS in HTML in TS". Not that it's blocking. I'd be happier if I got "JS in TS" though...

As for your snippet, there is nothing to fix there. The result is expected and correct. The first </script> closes the script tag in the outer HTML markup. It is more clear if we remove the outer JS layer and switch to HTML (unfortunately, GitHub syntax highlighter doesn't get it right):

Prettier pr-6038
Playground link

--parser html

Input:

<script>const html = /* HTML */ `<script></script>`;</script>

Output:

SyntaxError: Unexpected closing tag "script". It may happen when the tag has already been closed by another tag. For more info see https://www.w3.org/TR/html5/syntax.html#closing-elements-that-have-implied-end-tags (1:53)
> 1 | <script>const html = /* HTML */ `<script></script>`;</script>
    |                                                     ^

The inner closing script tag simply needs proper escaping:
UPD Oh! The escaping gets lost in formatting. Now, that's a bug.

Prettier pr-6038
Playground link

--parser babylon

Input:

const html = /* HTML */ `<script>const html = /* HTML */ \`<script><\\/script>\`;</script>`;

Output:

const html = /* HTML */ `
  <script>
    const html = /* HTML */ \`
      <script></script>
    \`;
  </script>
`;

As for the CHANGELOG, I thought minor bugs aren't reflected there, but okay, I'll do it.

@thorn0 thorn0 force-pushed the fix-nested-embeds branch from d42faff to fea4017 Compare May 23, 2019 16:47
@duailibe
Copy link
Copy Markdown
Collaborator

duailibe commented May 23, 2019

The result is expected and correct.

Thanks! My bad

Now, that's a bug.

Yikes, would you open a new issue? (or fix it here if you're interested)

As for the CHANGELOG, I thought minor bugs aren't reflected there

Yeah they are.. we only leave out changes that aren't visible to users

@thorn0
Copy link
Copy Markdown
Member Author

thorn0 commented May 23, 2019

would you open a new issue (or fix it here if you're interested)

I'll fix it here.

@duailibe
Copy link
Copy Markdown
Collaborator

@thorn0 I know we already handle something similar with GraphQL

@thorn0 thorn0 force-pushed the fix-nested-embeds branch from 7fcf3c9 to 9d591ea Compare May 24, 2019 00:46
@lydell
Copy link
Copy Markdown
Member

lydell commented May 24, 2019

@thorn0
Copy link
Copy Markdown
Member Author

thorn0 commented May 24, 2019

@lydell My two takeaways from that SO question:

  1. There is no generic (language-unspecific) way to escape the content of a script tag.
  2. It's recommended to escape HTML comments (the sequence <!-- ) inside scripts.

As for number one, since it doesn't exist, we probably aren't going to support it. :) Also this PR is specifically about JS in script tags, not about generic script tags.

As for number two, I never saw unescaped HTML comments in JS cause trouble. I'd be really curious to look at such a case. Looks like unneeded overcautiousness to me.

@thorn0 thorn0 force-pushed the fix-nested-embeds branch from 41356d4 to d38a699 Compare May 24, 2019 09:37
@duailibe duailibe merged commit e4f0df5 into prettier:master May 27, 2019
@thorn0 thorn0 deleted the fix-nested-embeds branch June 4, 2019 13:27
@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Sep 2, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Sep 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants