fix: correctly print double escaped backslashes with no raw value #56
Conversation
test/quotes.test.js
Outdated
| @@ -86,6 +86,42 @@ test('does not escape already-escaped single quotes', () => { | |||
| expect(code).toMatchInlineSnapshot(`"const str = 'a\\'b';"`); | |||
| }); | |||
|
|
|||
| test('correctly handle double escaped backslashes when theres no raw', () => { | |||
| // doing this since if there's a raw value it will prefer that | |||
There was a problem hiding this comment.
Maybe I'm misunderstanding your comment, but that's exactly why we have the clean function, that you are already calling. Haven't tested it, but I think you should be fine using load()
There was a problem hiding this comment.
Mmm the test was passing even before the changes when I called load...will check tho
There was a problem hiding this comment.
Interesting, you are right. But I don't know why 🤔 ....
I would have expected the following code to produce exactly the same AST as your manual way.
const ast2 = load("var text = $.text('\\\\');");But it's not. That's why you were unable to reproduce the issue.
Looks like acorn is doing some preprocessing while converting to the AST so that for whatever reason calling load() brings \\ as the value for the literal instead of \\\\.
So changing the input to ...
const ast = load("var text = $.text('\\\\\\\\');");... produces the error you were trying to reproduce. And your fix still fixes the issue 😆
That being said, I don't know what's less ugly to write in the test, I will leave that up to you
There was a problem hiding this comment.
Yup just explored the same and updated the comment...the value generated by acorn is just \\ while raw contains \\\\...I've even thought if we should match this in the svelte parse but tbf it feels more weird (maybe even buggish) what acorn is doing
🦋 Changeset detectedLatest commit: 9a077cc The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Closes sveltejs/svelte#15476
A bit of an edge case but a valid usecase nontheless.