Missing <script>s on test page #109
Conversation
wardpeet
force-pushed
the
85-missing-script
branch
from
4ca171f
to
fdc5c86
Compare
| var jsesc = require('jsesc'); | ||
|
|
||
| module.exports = (value) => jsesc( | ||
| value.replace(/[\r\n]{1,2}/g, '') |
There was a problem hiding this comment.
Why is the .replace needed?
There was a problem hiding this comment.
My original idea was to use jsesc(value, { wrap: true }) so it returns a string literal including the quotes.
There was a problem hiding this comment.
I think this needs .replace(/<\/script/g, '<\\/script>') since the output is placed inside a <script> in HTML.
My earlier question remains, though: why .replace(/[\r\n]{1,2}/g, ')'?
There was a problem hiding this comment.
You're correct, problem is that i thought jsesc was doing weird escaping but it was handlebars. I will push new commits later today.
wardpeet
force-pushed
the
85-missing-script
branch
from
fdc5c86
to
a72a9bf
Compare
|
Updated :) Let me know if it's okay :) |
| @@ -29,7 +29,31 @@ | |||
| </footer> | |||
|
|
|||
| {{#if benchmark}} | |||
| <script src="/test.js"></script> | |||
| <script src="/test.js"></script> <script src="/test.js"></script> | |||
|
I think we’re almost there! Left some comments. |
| { | ||
| wrap: true | ||
| } | ||
| ).replace(/<\/script/g, '<\\/script>'); |
There was a problem hiding this comment.
Let’s update jsesc to v1.1.0 and use escapeEtago: true instead of doing the .replace ourselves. 👍
wardpeet
force-pushed
the
85-missing-script
branch
from
a72a9bf
to
dcb1e7f
Compare
|
My tests didn't work so i didn't see some issues. But now they are fixed . I updated everything said above :) hopefully I finally got it right :p |
|
LGTM! Thanks, @wardpeet! |
Code with comments break, not sure how to fix this
should fix #85