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

unnecessary escaping in string concatinaion #440

Closed
EladBezalel opened this issue Feb 23, 2017 · 4 comments · Fixed by #490
Closed

unnecessary escaping in string concatinaion #440

EladBezalel opened this issue Feb 23, 2017 · 4 comments · Fixed by #490
Labels
bug Confirmed bug
Milestone

Comments

@EladBezalel
Copy link

Hey,
For some reason I can't explain babili(?) escapes the backslash in the following string:
input:

var x =  '\'cool\'' + 'test'

babili output:

var x='\\\'cool\\\'test';

it only happens when i concat strings, it works perfectly when i input:

var x =  '\'cool\'test'

I use the babili cli without any presets

p.s the babel repl doesn't show the problem (probably browser html escaping)

@Jamesernator
Copy link

I had the same issue, its not actually the backslash being quoted its the single quote, e.g. this is a simpler breaking case:

// In code
const s = " ' " + "quote breaks"

// Out code
const s = " \\' quote breaks"

@goto-bus-stop
Copy link
Contributor

goto-bus-stop commented Mar 26, 2017

Hmm I guess after #384 the strings are first run through jsesc by the minify-constant-folding transform, and then again by babel-generator. So backslashes end up being duplicated.

Maybe instead of using jsesc, minify-constant-folding should handle </script and <!-- manually? Or babel-generator could pass isScriptContext to jsesc, then babili doesn't need to worry about that either.

@boopathi
Copy link
Member

What would be a correct way to handle these ?

/cc @mathiasbynens

@mathiasbynens
Copy link
Contributor

As @goto-bus-stop said: don‘t escape strings twice.

The most sensible solution IMHO would be to make babel-generator pass isScriptContext: true to jsesc and to remove the additional/faulty escaping from minify-constant-folding.

@boopathi boopathi added the bug Confirmed bug label Apr 3, 2017
boopathi added a commit that referenced this issue Apr 3, 2017
+ Adds option isScriptContext to constant folding plugin
+ Fix #440, fix #413
+ Related #384, #382
@boopathi boopathi mentioned this issue Apr 3, 2017
boopathi added a commit that referenced this issue Apr 4, 2017
+ Adds option isScriptContext to constant folding plugin
+ Fix #440, fix #413
+ Related #384, #382
boopathi added a commit that referenced this issue Apr 6, 2017
* Make jsesc isScriptContext optional

+ Adds option isScriptContext to constant folding plugin
+ Fix #440, fix #413
+ Related #384, #382

* Remove jsesc
@boopathi boopathi added this to the 1.0 milestone Apr 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants